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

Use TSL to generate TSL strings? #23

Open
cben opened this issue Apr 30, 2020 · 6 comments
Open

Use TSL to generate TSL strings? #23

cben opened this issue Apr 30, 2020 · 6 comments

Comments

@cben
Copy link

cben commented Apr 30, 2020

One drawback of exposing an API accepting a pseudo-SQL query strings (parsed by TSL) is that it's tricky for clients to generate those strings with correct value quoting... We have some ad-hoc string-building code and I'm looking at more principled generation from something like an AST. In Go.

  • I first looked at squirrel but turns out it can't generate a self-contained SQL string. It only emits ? placeholders, and returns a separte array of values, assuming you have a DB interface that safely takes separate template + args.
    This is good design but our API doesn't support that 😁, it takes a single TSL string.

  • So my next idea was using TSL itself to generate a TSL string! 💡
    Ironically, currently sql walker uses squirrel as well, emitting ? and separate values 😁

Is this something you'd like like TSL to support — round trip from AST to a TSL string that parses to same AST? 🎋 -> 🔤 -> 🎋

  • Note that for sending SQL queries to actual databases, it may still be best to emit placeholders + separate values. Emphasis on plural databases — one of the reasons Go stdlib haven't implemented string quoting is that quoting rules differ by database 🤦
    This probably means having separate SQL and TSL walkers. Maybe some clever polymorphism with Sqlizer interface is possible, not sure how.
@yaacov
Copy link
Owner

yaacov commented Apr 30, 2020

Interesting 👍

are you suggesting a builder that will help building tsl-tree from code, instead of parsing tsl-phrases ?

e.g. something to automate this ?

right := Node{Func: StringOp, Left: "Moki"}
left := Node{Func: StringOp, Left: "Not Moki"}

tslTreeRoot := Node(Func: EqOp, Right: right, Left: left}

@cben
Copy link
Author

cben commented Apr 30, 2020

I'm suggesting a walker [or a flag on existing sql walker] that from the example you give above, would build the self-contained string "Moki" = "Not Moki".
My understading is that currently it builds ? = ? plus the args "Moki", "Not Moki", right?

@yaacov
Copy link
Owner

yaacov commented Apr 30, 2020

My understading is that currently it builds ? = ? plus the args "Moki", "Not Moki", right?

yes,

so filing in the args in a string with '?' place holders sounds like for each arg do strings.Replace of ? ?

@cben
Copy link
Author

cben commented Apr 30, 2020

Interestingly, squirrel.Expr may provide the infrastructure to "smuggle" literals into squirrel:
https://github.com/Masterminds/squirrel/blob/master/expr.go#L31-L82
If any of the args supports .ToSql(), it inlines the result in place of the ?.

@yaacov
Copy link
Owner

yaacov commented Apr 30, 2020

sql walker return Sqlizer so it should supports .ToSql() out of the box ... !?

@cben
Copy link
Author

cben commented Apr 30, 2020

OTOH, I don't understand how this now works:

	case tsl.StringOp:
		s = sq.Expr(n.Left.(string))

Isn't sq.Expr() supposed to take an SQL fragment (with possible placeholders)? Here you're feeding it the string as-is?
Playing with the tests, but I'm not managing to break it the way I expected 👍, so I guess it's time to call it a night :)

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