镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

[jdbc-v2] parser doesn't recognize aliases that happen to be keywords #2427

Closed

Description

@Gerrit-K

Description

@chernser: The real problem is in statements like:

SELECT [...] FROM some_database.some_table source
WHERE source.some_column = ? AND source.some_other_column = ?

We need to make sure that short form of alias do not break parameter detection.

-- original description
The JDBC-V2 parser doesn't seem to correctly parse statements where there are aliases that happen to be keywords. The examples below illustrate this issue.

Steps to reproduce

  1. Using source as an alias fails:
    PreparedStatement stmt = conn.prepareStatement("SELECT date, ? FROM system.one source WHERE ? = ?");
    conn.getParameterMetadata().getParameterCount(); // returns 1 instead of 3
  2. Prepending AS before the alias fixes the issue:
    PreparedStatement stmt = conn.prepareStatement("SELECT date, ? FROM system.one AS source WHERE ? = ?");
    conn.getParameterMetadata().getParameterCount(); // returns 3 as expected
  3. Using a different alias also fixes the issue:
    PreparedStatement stmt = conn.prepareStatement("SELECT date, ? FROM system.one _source WHERE ? = ?");
    conn.getParameterMetadata().getParameterCount(); // returns 3 as expected
    

Expected Behaviour

The example query without the AS before the alias should work as well, as it's valid ClickHouse syntax.

Code Example

see repro steps above

Environment

Running integration tests on v0.8.6

Activity

  1. chernser commented on Jun 5, 2025

    @chernser
    Contributor

    Thank you for reporting, @Gerrit-K !
    We will look into it.

  2. added this to the 0.9.0 milestone on Jun 5, 2025
  3. chernser commented on Jun 5, 2025

    @chernser
    Contributor

    @Gerrit-K This is invalid for JDBC - prepared statement may replace only data values.

  4. Gerrit-K commented on Jun 6, 2025

    @Gerrit-K
    Author

    @chernser Thanks for looking into this! I'm not sure if I understand your comment, though 🤔 This issue is not particularly about prepared statements (although they are affected by this), but rather about a bug in the parser - specifically around the AS source part of the query. When you omit the AS, it seems to take source as a keyword instead of an alias, which causes the parser to fail. Using a different alias (_source or foo) works fine.

    If used in a prepared statement, this means that all parameters (?) that come after the token source will not be detected, hence the count 1 instead of 3.

    This is the very first case I was stumbling upon yesterday and it's a pretty important one for us. We use the jdbc driver indirectly via Metabase and a lot of queries on our production instance are written like this:

    SELECT [...] FROM some_database.some_table source
    WHERE source.some_column = ? AND source.some_other_column = ?

    Since the driver cannot detect the parameters, all of these queries failed after upgrading to the newest driver version, so we needed to downgrade again and need to wait for a fix before attempting another upgrade.

  5. removed this from the 0.9.0 milestone on Jun 6, 2025
  6. chernser commented on Jun 6, 2025

    @chernser
    Contributor

    @Gerrit-K I've re-opened the issue and added root problem. Will work on it next week.

    Where this example SELECT date, ? FROM system.one AS source WHERE ? = ? came from? Do you have queries that try to use parameters in place of columns?

    Thanks!

  7. added this to the 0.9.0 milestone on Jun 6, 2025
  8. chernser commented on Jun 6, 2025

    @chernser
    Contributor

    @Gerrit-K I think not much can be done here because not every keyword work as alias on server side:

    5aa685a6fae7 :) select number from system.numbers join limit 1
    
    Syntax error: failed at position 46 (1):
    
    select number from system.numbers join limit 1
    
    Expected one of: token sequence, Dot, token, OpeningRoundBracket, UUID, alias, AS, identifier, FINAL, SAMPLE, USING, ON, end of query
    
  9. removed this from the 0.9.0 milestone on Jun 6, 2025
  10. chernser commented on Jun 6, 2025

    @chernser
    Contributor

    @Gerrit-K would you please list all keywords you are using as alias?
    I will then check them with server and add to exception.
    I fixed the source.

    Thanks!

  11. Gerrit-K commented on Jun 6, 2025

    @Gerrit-K
    Author

    @chernser thanks for reopening this.

    Where this example SELECT date, ? FROM system.one AS source WHERE ? = ? came from? Do you have queries that try to use parameters in place of columns?

    To be honest, this was just a constructed example to have something very simple that works in ClickHouse, but reproduces the bug I was running into. I specifically wanted something where there was at least one parameter before and one after the source, so that I could verify that it's really his token that breaks the flow. In the real use case, it was a very large, but not exotic query, that can very roughly be reduced to:

    SELECT source.mycolumn FROM mytable source WHERE mycolumn = ?

    Due to the source in there, the parser wasn't able to find the ? parameter.

    I think not much can be done here because not every keyword work as alias on server side

    Interesting! But makes sense.

    would you please list all keywords you are using as alias?

    That would be very difficult for me, as we have tens of thousands of persisted queries in our BI tool, and I would have to parse the SQL for each of them to find out what aliases are used (and for that I would need a working ClickHouse SQL parser 😅). Unfortunately, that would take far too much effort. However, allowing source is certainly a good start, as this sounds like a common case semantically. I can't currently think of any other keywords that our users might mistakenly use as an alias.

    However, all of this made me wonder: Surely ClickHouse must already have some kind of internal grammar implementation to parse incoming queries, right? Couldn't that be reused somehow, either directly, or at least as a blueprint for an adapted Antlr4 grammar?

  12. chernser commented on Jun 6, 2025

    @chernser
    Contributor

    @Gerrit-K as far as I know ClickHouse has own parser but provides Antlr4 grammar. We use it but have to adjust for JDBC needs and it complicates things a bit.
    I have added source and it works now. I will look through keywords and will check what may look a good alias name.

    As my personal thought: having short form of alias may be not portable to other DB and avoiding it in the future at least would be a good thing.

  13. Gerrit-K commented on Jun 6, 2025

    @Gerrit-K
    Author

    ClickHouse has own parser but provides Antlr4 grammar. We use it but have to adjust for JDBC needs and it complicates things a bit.

    I see, that makes it at least not feasible to have a 1:1 copy.

    As my personal thought: having short form of alias may be not portable to other DB and avoiding it in the future at least would be a good thing.

    I completely agree with you! It would have probably never happened if ClickHouse wouldn't allow this in the first place 😅 Or if the previous version of the JDBC driver didn't allow it. But it did, and so all of our users created these queries that are now suddenly incompatible.

    I have added source and it works now. I will look through keywords and will check what may look a good alias name.

    Thanks a lot! Much appreciated 🙏

  14. added this to the 0.9.0 milestone on Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions