Statement Parameters are not handled correctly

[BramGruneir]

The common ones that I can think of are:

• ?
• $0, $1 ...
• %0, %1 ...

I guess, it's probably best to handle ?, $, % with and without numbers.

e.g.

SELECT * FROM pg_catalog.pg_foreign_table WHERE ftrelid = ?;

[maddyblue]

What's the motivation for this? Pg/CRDB don't support ? afaik. sqlfmt has a limited goal of parsing PG SQL.

[BramGruneir]

Query rewriting when coding and eventually integration within an editor. I get a ton of queries that are destined not to be ingested directly, but instead used in coding. I always want those formatted correctly.

Golang's db package uses ? for example:

updateMoney, err := db.Prepare("UPDATE balance SET money=money+? WHERE id=?")
...
tx, err := db.Begin()
...
res, err := tx.Stmt(updateMoney).Exec(123.45, 98293203)

This is from here: https://golang.org/pkg/database/sql/#Tx.Stmt

[maddyblue]

I guess my point is that currently, sqlfmt's goal is to format PG SQL. ? is not valid in postgres, so sqlfmt also doesn't think it's valid. Supporting all or more forms of SQL changes the goal of sqlfmt. Maybe that's ok but it would bump the complexity up at least a little.

[BramGruneir]

If I have some spare cycles, I might hack away and send you a PR. This hurts the usefulness of the website when coding.

[maddyblue]

Ok. Out of curiosity, how do you expect this to work? We'd have to teach the cockroach parser about ?. Maybe we could change the placeholder datum type to also take a placeholder character so it can parse and print them correctly? @knz thoughts?

[BramGruneir]

I honestly haven't looked at it all, but yes, that was the idea.

[knz]

This is going to break horribly. What you really need to do is to filter the query through the Go text subtitution for question marks (which may live in lib/pq?) and the get the placeholders out of that, then parse that with sqlfmt.

[knz]

So the detail of why it will break:

• the usage of a ? for placeholders is not part of the pgwire protocol / pg SQL syntax. It is the responsibility of the client driver/framework to perform this substitution.
• when the developer of a client driver/framework makes a mistake and does not perform this substitution correctly, it is the responsibility of the pg/crdb server to report an error ("syntax '?' not recognized" or similar) to explain to the client programmer that they made a mistake
• if we were to change crdb to silently accept this syntax for whatever reason, we'd start silently accepting bogus requests by erroneous client drivers. Who knows what hell would ensue (including potentially insecure data being processed by our engine...)

[knz]

I also forgot to mention that a standalone ? is a valid SQL operator now (it's a JSON operator).