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

Parameterizing #:order-by #1

Open
greghendershott opened this issue Jul 11, 2017 · 1 comment
Open

Parameterizing #:order-by #1

greghendershott opened this issue Jul 11, 2017 · 1 comment

Comments

@greghendershott
Copy link

greghendershott commented Jul 11, 2017

First, this is a fantastic library -- thanks! I only discovered it yesterday so I'm still wrapping my head around it, but the documentation is really good.

The only wall I've hit so far:

I'd like to use placeholders for #:order-by. (Motivation: Imagine a query that wants to reflect a user clicking column headers.)

But trying to use

#:order-by ,order-by-column

gives an error because the SelectOrderItem syntax pattern uses ScalarExpr:

(define-splicing-syntax-class SelectOrderItem
  #:attributes (ast)
  (pattern (~seq e:ScalarExpr o:SelectOrderDirection)
           #:attr ast (select:order ($ e.ast) ($ o.dir))))

whereas later emit-select-order uses emit-name:

    (define/public (emit-select-order so)
      (match so
        [(select:order column asc/desc)
         (J (emit-name column) ;; <--- should be (emit-scalar-expr column) ???
            (case asc/desc
              [(asc) " ASC"]
              [(desc) " DESC"]
              [(#f) ""]))]))

In addition, I'm not sure how to specify #:asc or #:desc keywords for the direction, using placeholders?


I tried hacking the latter instead to use emit-scalar-expression. This way, I can at least do something like

#:order-by (ScalarExpr:INJECT ,(format "~a ~a" order-by-column order-by-direction))

and "it works". Albeit using INJECT isn't ideal.


Is this pilot error? Or if there's a bug/omission, is my change a reasonable fix I should PR, or a bad hack?

Thanks!

@rmculpepper
Copy link
Owner

You're right, that code should use emit-scalar-expr instead of emit-name. I'll push the change.

Placeholders can only stand for SQL scalar constants, not expressions or things like order directions. Abusing ScalarExpr:INJECT is probably the best solution for now.

Longer-term, I could imagine adding more dynamic AST manipulation support and more nonterminal escapes, so you could write something like

#:order-by (SelectOrderItem:AST
            ,(make-SelectOrderItem (make-Ident column-name)
                                   (if asc? 'asc 'desc)))

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

No branches or pull requests

2 participants