[PostgreSQL]PostgreSQL client should recognize ? placemarkers

[BillyYccc]

Postgres' $1, $2, etc, is nonstandard, AFAIK, and contrary to ODBC and JDBC.

We should at least have the option to use this syntax if we want to.

Issue reported in smallrye/smallrye-mutiny-vertx-bindings#106

[julianladisch]

There is no standard.
Oracle uses :a, :b, :c, etc. syntax: https://docs.oracle.com/cd/B28359_01/appdev.111/b28370/dynamic.htm#BJEIJEBJ
PHP PDO supports both named (:name) or question mark (?) parameter marker placeholder: https://www.php.net/manual/en/pdo.prepare.php
vertx-pg-client neither supports ODBC nor JDBC.
Supporting ? placeholders requires ? masking and thus is a breaking change: "In JDBC, the question mark (?) is the placeholder for the positional parameters of a PreparedStatement. There are, however, a number of PostgreSQL operators that contain a question mark. To keep such question marks in a SQL statement from being interpreted as positional parameters, use two question marks (??) as escape sequence." https://jdbc.postgresql.org/documentation/head/statement.html
For these reasons I do not support this feature request (unless there is an configuration option to switch from dollar to question mark placeholders).

[vietj]

Agreed that out of the box we should stick with original DB syntax.

I would like however to support named place holders for simple mapping (i.e query("INSERT INTO foo (firstName, lastName) VALUES (':firstName', ':lastName')", new JsonObject().put("firstName", "foo").put("lastName", "bar"))) that would allow doing simple pojo mapping based on databinding.

[gavinking]

Folks, I would still very much like to see this.

We really have to jump through hoops in Hibernate Reactive to accommodate this. I didn't want to push the issue too hard earlier, since, well, HR was still in a very experimental stage.

But do still I think it's a really pretty basic feature of a client API to abstract over differences in the parameter-binding syntax of the various supported databases.

There is no standard.

That's simply not true. JDBC and ODBC are almost-universally-supported standards. It's in our very-strong interest to make it easy for people to take SQL they wrote for execution via JDBC, and have it work without change on Vert.x.

[jrno]

This proved out to be a slight inconvenience for me when exploring the option to run jOOQ generated sql against the vertx-pg-client. They seem to only support jdbc style or named bindings out of the box (https://www.jooq.org/doc/latest/manual/sql-building/bind-values/)

[vietj]

JDBC and ODBC are almost-universally-supported standards in Java

not strongly against it, but just giving some perspective

[vietj]

it is not clear what is needed from HRX perspective, do you mean that HRX will generate queries with placeholder and then the client will parse it to replace the placeholders by the native placeholders or do you mean more ?

If that's the former we can easily do it by providing some kind of query builder that will generate a portable string accross vendor, i.e something like new Query().appendSql("SELECT * FROM TABLE WHERE ID=").appendParameter().toString() that generates SELECT * FROM TABLE WHERE ID=$1 for PG, SELECT * FROM TABLE WHERE ID=? for MySQL, etc...

If that's the later it is more difficult to achieve and might lead to parse each database syntax which is not a goal of this project, i.e https://github.com/pgjdbc/pgjdbc/blob/161ea24965b3d11067f96b9765cda10f8b59e08b/pgjdbc/src/main/java/org/postgresql/core/Parser.java#L45 unless we provide a syntax that does not require to parse SQL

[gavinking]

do you mean that HRX will generate queries with placeholder and then the client will parse it to replace the placeholders by the native placeholders

Right, that.

If that's the former we can easily do it by providing some kind of query builder that will generate a portable string across vendor

No that doesn't help because it's not HR generating the SQL. We're just taking SQL that Hibernate generates and doing this same replacement ourselves.

And of course we can do that (are doing it, in fact).

But see the thing is, that it's not just HR that needs to do this: it's anyone who wants to use any sort of pre-existing SQL-generation lib written for Java together with the SqlCient, along with people who have oodles of hand-written SQL.

You would save uses a lot of pain by just centralizing that code in each of the Vert.x drivers.

If that's the later it is more difficult to achieve and might lead to parse each database syntax

Right, but right now you're forcing lots of clients to do that reparsing for you. And they're probably going to do a worse job of it.

But let me make a small correction here: you definitely don't need to parse anything. You merely need to tokenize the incoming SQL, which a task that really is very straightforward. Building a parser for SQL is quite a task; building a tokenizer for SQL is pretty trivial.

[gavinking]

JDBC and ODBC are almost-universally-supported standards in Java

No, not just in Java! ODBC is supported across multiple languages. And JDBC is just a Java clone of it.

[vietj]

you are saying that a simple tokenization of a query with ? as delimiter will do the job, however this thread indicates it seems to be more complex than that https://stackoverflow.com/questions/14779896/does-the-jdbc-spec-prevent-from-being-used-as-an-operator-outside-of-quotes , WDYT ?

[gavinking]

What in particular in that thread are you referring to?

I'm not asking you to support the ODBC/JDBC escape syntax, since that is really quite complex, with lots of standard functions to translate to database native functions, and not even JDBC drivers do a good job of supporting it.

All I'm asking for is the query parameter syntax.

[vietj]

It is not clear to me actually what you are asking to support.

Are you asking that the PostgreSQL client is able support rewriting queries using ? as placeholders instead of $n sequence by using a trivial ? delimited tokenization ?

[gavinking]

Are you asking that the PostgreSQL client is able support rewriting queries using
? as placeholders instead of $n sequence by using a trivial ? delimited tokenization ?

Yes. But the tokenization is not quite that trivial since you need to avoid doing replacement inside comments, quoted identifiers, and literal strings.

[gavinking]

And the point is that I want to call some method where that just happens automatically if it is the Postgres client, without having to ask myself what client it is.

[pmlopes]

Just for better understand the problem. HRX generates SQL from java code/classes, and the generated code is aware of the target database (for type support, sequences, etc).

So what is blocking HRX to generate the SQL with the target specific marker? IMHO this looks like asking for a change on the wrong side of the problem.

I have the feeling this would be wasting a few CPU cycles as time was spent on generating SQL on HRX then the client, needs to rewrite the generated SQL to adhere to the server.

Sorry for not adding anything to the thread, but just expressing my opinion.

[gavinking]

HRX generates SQL from java code/classes, and the generated code is aware of the target database (for type support, sequences, etc).

No, HR doesn't generate SQL. Hibernate ORM generates SQL. And, like essentially every other library that generates SQL in the entire Java ecosystem, it generates SQL with ? placeholders.
HR is a separate library that just forwards that generated SQL to a SqlClient.

Now, there are two choices here. Either:

1. HR, along with every single other client with existing hand-written SQL, or which uses an existing library to generate SQL has to write a function which does something special for the Postgres SqlClient, or alternatively
2. we write this function once, and everyone can reuse it.

I really don't quite comprehend the resistance to this. It's a relatively-trivial function that just makes the Vert.x database drivers work the same way as every JDBC and ODBC driver across multiple languages and all SQL databases.

It's totally the norm for database drivers to do this.

[gavinking]

So what is blocking HRX to generate the SQL with the target specific marker?

In fact this is the path we tried to go down, and after a lot of suffering, we essentially failed to make it work. You see, there are lots of places in Hibernate ORM which render SQL, and sometimes different "fragments" are rendered in different places, and then there are a few other places where already-rendered SQL (or hand-written SQL passed by the user) is subsequently manipulated in some way (to add limits, filters, etc). And all these different bits need to be aware of each other in order to generate the right parameter numbers and not wind up with where x=$1 and y=$1.

(To be clear, what makes this problem painful isn't $ vs ?, it's that the $s have numbers attached to them, which requires coordination between different bits of Hibernate.)

So anyway, now I've just given up on this and I've decided to rip out all that code and just post-process the SQL and replace the ?s right before handling it off to the Vert.x driver. Which is fine. But it would be much better if the function that does this were in the Postgres SqlClient.

[gavinking]

So to clear things up a bit, here's the implementation I'm currently using. I'm sure it can be improved. Perhaps it's going to exhibit better performance by working directly with bytes instead of codepoints, for example, and I know it should support Postgres $$ quotes.

But it's really not that much of a scary thing, it seems to me.

[vietj]

@gavinking yes I see what you mean, I think we can do such API but keep it internal (i.e on SqlClientInternal interface) for HRX as I don't want encourage API users to use it. The main reason is that it seems simple explained like this but it might not be and require to invest a lot of time and effort in which case we cannot support such API.

[vietj]

I think we could have a tag on SqlClientInternal that will internally rewrite SQL queries to use something like the code you provided when set

[gavinking]

I think we can do such API but keep it internal ... for HRX as I don't want encourage API users to use it.

Well then there's barely any value in having it in Vert.x rather than in HR, since the whole point of why I'm asking for this is that I think other people will want to use it. I find it difficult to believe that we're the only ones who're going to run into this.

I think we could have a tag on SqlClientInternal that will internally rewrite SQL queries to use something like the code you provided when set

Ideally from my point of view it would be a setting on ConnectOptions or PoolOptions that let me setPortableParameterSyntax(true).

[julianladisch]

@jrno jOOQ supports all bind parameter syntaxes:

• ? (JDBC, MySQL, MariaDB)
• @name (SQL Server)
• :name (Oracle, Spring, JPA named syntax)
• ?1 (JPA indexed syntax)
• $1 (PostgreSQL)
• @1 (SQL Server)

Example for PostgreSQL:

    Settings settings = new Settings().withRenderNamedParamPrefix("$").withParamType(ParamType.NAMED);
    DSLContext create = DSL.using(SQLDialect.POSTGRES, settings);
    Select<Record> select = create
        .select()
        .from(table(name("item")))
        .where(field(name("id")).eq(val("foo")));
    System.out.println(select.getSQL());
    System.out.println(select.getBindValues());

Output:

select * from "item" where "id" = $1
[foo]

In general the database abstraction or code generator (jOOQ, hibernate, JDBC) should output the database specific syntax. There should be not need for a plain database driver to tokenize the SQL for syntax convertion. vertx-sql-client is a plain database driver, not a JDBC driver with database abstraction.

[julianladisch]

@gavinking Hibernate System Requirements:

Hibernate 5.2 and later versions require at least Java 1.8 and JDBC 4.2.

vertx-sql-client/vertx-pg-client is not a JDBC driver.

hibernate-orm has SQLQueryParser#substituteParams(String sqlString) that converts ?1 and :name placeholders to ? placeholders:
https://github.com/hibernate/hibernate-orm/blob/5.4.18/hibernate-core/src/main/java/org/hibernate/loader/custom/sql/SQLQueryParser.java#L283
https://github.com/hibernate/hibernate-orm/blob/5.4.18/hibernate-core/src/main/java/org/hibernate/engine/query/spi/ParameterParser.java
This code could be adopted to convert placeholders to $1 placeholders, similar to the jOOQ placeholder conversion mentioned above. That way hibernate-reactive could produce the SQL syntax needed for vertx-pg-client.

[Yaytay]

In my case the SQL is written by humans.
Sometimes they won't know which database engine their query will run against and (more often) I don't want them to have to care.
I don't want to remove the ability to use the native parameter placeholder and I don't want to remove the ability to have provider-specific SQL, but I would like my users to have the option of translating a (standard ANSI) query from a generic form to the provider-specific form.
I would, separately, also like the option of having named (or indexed) parameters in a consistent way.

So my proposal would be to have two functions, one that converts JDBC placeholders to native placeholders and another that converts named placeholders to native placeholders.
These functions would also do 'whatever it takes' to permit lists/arrays to be used as parameters to an 'in ()' clause (which could involve manipulating the argument list as well as the SQL).
The functions would need to be more clever than a search and replace, but for me they don't need to be too clever because the user always has the option of going native.

I'm quite happy to work on creating these functions, but I can only test against MS, My & Postgres.
Also, some help will be needed from the libraries to inform the functions as to what the native placeholders are (and whether they are indexed, and how the native library handles arrays).