Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unable to run an arbitrary query #246

Closed
damianham opened this issue Jun 26, 2018 · 3 comments · Fixed by #247
Closed

unable to run an arbitrary query #246

damianham opened this issue Jun 26, 2018 · 3 comments · Fixed by #247

Comments

@damianham
Copy link
Contributor

Previously this code worked fine:

sections = [] of String
    Post.query("select distinct section from posts") do |rs|
      rs.each do
        sections.push(rs.read(String))
      end
    end

With version 0.12.1 this no longer compiles and produces this error output

instantiating 'PostController#index()'
in src/modules/post/post_controller.cr:20: macro didn't expand to a valid program, it expanded to:

================================================================================
--------------------------------------------------------------------------------
   1.     @@select.custom = "select distinct section from posts"
   2. 
   3.     def self.select
   4.       @@select.custom
   5.     end
   6.   
--------------------------------------------------------------------------------
Syntax error in expanded macro: query:3: can't define def inside def

    def self.select
    ^

================================================================================

    Post.query("select distinct section from posts") do |rs|
         ^~~~~
@drujensen
Copy link
Member

@noahlh @robacarp @damianham it looks like we defined query twice. We need to change one of them.

The original definition provides a way to perform direct database queries as @damianham shows above.

https://github.com/amberframework/granite/blob/master/src/granite/querying.cr#L115

We support pass throughs for exec, query and scalar to allow direct db access.

@noahlh recently added a new macro query to define a custom query to provide a view object commonly seen in other orm libraries:

https://github.com/amberframework/granite/blob/master/src/granite/select.cr#L10

This was originally supposed to be called select instead of query which is a better name but @faustinoaq pointed out this conflicts with crystals select reserved word.

I recommend we consider changing the macro name to select_statement or if we want something shorter sql. Are there other suggestions on a new name for the macro?

wdyt?

@drujensen
Copy link
Member

created a PR #247 to rename it to select_statement.

@noahlh
Copy link
Contributor

noahlh commented Jun 27, 2018

Good catch, and sorry about the naming conflict! Thanks for the quick fix @drujensen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants