rmculpepper / sql

Embedding of some of SQL into Racket
33 stars 5 forks source link

Support dynamic insertion of SQL identifiers and names. #10

Closed LiberalArtist closed 5 years ago

LiberalArtist commented 5 years ago

This commit extends the grammar for ident and name to include (Ident:AST (unquote ast-racket-expr)) and (Name:AST (unquote ast-racket-expr)), respectively, where the inserted AST value must satisfy the corresponding (pre-existing) predicate, i.e. ident-ast? or name-ast?.

The motivation is to make it possible to abstract over the table name in INSERT, UPDATE, DELETE, and CREATE TABLE statements, though the additions should be more generally useful. For example, the following function now works as expected:

(define (delete* table)
  (delete #:from (Name:AST ,table)))

(delete* (ident-qq myTable))

Writing such a function was not possible before, as the term following #:from is restricted to be a table-name, not an arbitary table-ref.

There is an oddity in the interaction between the name nonterminal and the table-ref nonterminal due to the internal representation. Given an ident AST value:

(define id
  (ident-qq myTable))

the id value will not satisfy table-ref-ast?, and attempting to use it as a table-ref (e.g. (select col #:from (TableRef:AST ,id))) will raise an exception.

A workaround is needed to make the library wrap the value in a table-ref:name struct:

(define ref
  (table-ref-qq (Name:AST ,id)))

(table-ref-ast? ref)

(select col #:from (TableRef:AST ,ref))

It would seem desirable to investigate adjusting the internal representations to make values that satisfy name-ast? also satisfy table-ref-ast?. Alternatively, the workaround could be documented.

Incremented package version to "1.1.1".

rmculpepper commented 5 years ago

Thanks! This looks good. Can you change the version to "1.2" instead of "1.1.1"? I don't think there's a need for three-level version numbers.

Regarding the AST is-a issue, I'm worried about creating an overlap between ScalarExpr and TableExpr/Ref, so I think it would be better to describe the workaround for now. There are some other cases that might need straightening out, though; for example using a Select as a TableExpr.

LiberalArtist commented 5 years ago

Ok, I've changed to "1.2". I also added history annotations for everything changed since "1.1".

I see what you mean about the AST issues. I think it makes sense to defer addressing that for now.

rmculpepper commented 5 years ago

Merged, thanks!