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

Allow ? for parameters #1987

Closed
linux-china opened this issue Feb 28, 2023 · 8 comments
Closed

Allow ? for parameters #1987

linux-china opened this issue Feb 28, 2023 · 8 comments

Comments

@linux-china
Copy link
Contributor

linux-china commented Feb 28, 2023

Just noticed prql-jdbc driver from #1929 and it's really useful. Just send PRQL over prql-jdbc and prql-jdbc will transpile PRQL to SQL transparently and fetch data from database.

A question here, Java JDBC/R2DBC use ? as placeholder for parameters, and now PRQL uses $1, $2 as parameters, and maybe Allow braces for parameters, but ? as parameter placeholder still very important because it's a standard in JDBC.

An example for prql-jdbc:

PreparedStatement stmt = conn.prepareStatement("from mytable | filter name == ?");
//  SELECT * FROM mytable WHERE name = ?
stmt.setString(1, "John");

With prql-jdbc and ? as parameter placeholder, and I think most Java JDBC frameworks/libraries can use PRQL directly:

  • Spring JdbcTemplate:
String query = "from EMPLOYEE | filter ID == ?";
// "SELECT * FROM EMPLOYEE WHERE ID = ?";
Employee employee = jdbcTemplate.queryForObject(
  query, new Object[] { id }, new EmployeeRowMapper());
  • MyBatis
<select id="selectPerson" parameterType="int" resultType="hashmap">
  from PERSON | filter ID == #{id}
</select>
<!--  BoundSql:  select * from PERSON where id = ? --> 
@max-sixty
Copy link
Member

Thanks for the issue @linux-china .

Are there any docs on the use of ?? For example, I'm curious how it deals with multiple parameters.

My initial hypothesis is that we should use one thing in PRQL and then the integrations can do the translation — for example if we decide to use ${foo}, then the JDBC driver can translate that to ? if needed. But let's have a look at some cases and see what would work best.

(And if you have any feedback on #1929, I'm sure it will be well-received. I notice you already made an addition to the existing code to throw an error in Java rather than panic; that sort of thing might be useful advice in #1929 too)

@linux-china
Copy link
Contributor Author

linux-china commented Feb 28, 2023

Javadoc for PreparedStatement: https://docs.oracle.com/javase/8/docs/api/java/sql/PreparedStatement.html

JDBC spec only supports positional parameter with placeholder ?. But positional parameter is some risky, and some Java frameworks/libraries use named parameters.

        String sqlQuery = "INSERT INTO student VALUES(:id, :name, :department)";
Mono.from(connectionFactory.create())
  .flatMapMany(connection -> connection
    .createStatement("SELECT firstname FROM PERSON WHERE age > $age")
    .bind("$age", 42)
    .execute())
    @PRQL("from user|filter id == ${id}")
    User findUserById(Integer id);

or you can wrapper JdbcTemplate to PrqlTemplate:

prqlTemplate.query("from employee | filter name == ${name}", ... )

we decide to use ${foo}, then the JDBC driver can translate that to ?

My option: ? is for JDBC level and it's fundamental, Named parameter ${name} or :name is for frameworks and libraries. Sometime we should use both. For example, if you use Spring Framework, almost you will choose named parameter because less risky and easy to write.

@aljazerzen
Copy link
Member

But if some Java frameworks/libraries use named parameters, they must be translated to positional parameters before they hit the database. Because I know for a fact, that Postgres does not support named parameters.

Could these frameworks/libraries translate named to positional before we compile PRQL to SQL?

This way PRQL as a language does not need to support many ways to do the same thing.

@linux-china
Copy link
Contributor Author

Could these frameworks/libraries translate named to positional before we compile PRQL to SQL?

I'm not sure. A easy solution now is to wrap up these frameworks/libraries to accept PRQL, then compile PRQL to SQL, and finally call delegated APIs that accept SQL. With this way, you just need to replace named PRQL parameter from ${name} to :name framework style, and then the frameworks will handle all others things.

The following API is to wrap up Spring NamedParameterJdbcTemplate with PRQL support.

    @Test
    public void testParameters() throws Exception {
        prqlTemplate.query("from test2 | filter id == ${id}", Collections.singletonMap("id", 1), rs -> {
            System.out.println(rs.getString("name"));
        });
    }

To wrap up Spring JdbcTemplate, and it's easy, and now I can use PRQL directly to query database by wrapping up Spring JdbcTemplate.

image

@zhicwu
Copy link

zhicwu commented Feb 28, 2023

Good point @linux-china. I was thinking to add a Java parser which can address issue like this.

Anyway, I think it's probably better to add an compile option in PRQL lib to recognize parameters based on given pattern.

@aljazerzen
Copy link
Member

Wow @linux-china this is great! Are you using just the plain library from main branch or the PR for JDBC?

But regarding using ? or :some_name for params - I don't think this is possible to parse consistently.

Take this example:

from a
select [x ? 5]
select :param

In the first select, ? will be parsed as coalesce, even though you may want it to be a param (and x is a function that takes ? and 5).

In the second select, :param will ne parsed as a named param to select function, even though it you want to be a query parameter.


Sidenote: #1957 adds support for params of form $some_name.


It seems that the only way to make JDBC and other frameworks / libraries work correctly, would be to compile PRQL to SQL after ? and :name have been processed into $1.

@linux-china
Copy link
Contributor Author

MyBatis with PRQL:

image

? parameter is required.

@snth
Copy link
Member

snth commented Jul 10, 2023

See #1479 for how to work around this with s-strings.

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

5 participants