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

Parsing fails if quoted table name is a reserved word (prepared statement) #10739

Open
iancamp opened this issue Jun 1, 2018 · 5 comments
Open

Comments

@iancamp
Copy link

iancamp commented Jun 1, 2018

I initially encountered the following issue using prestodb version 0.193 in a docker container. I have verified the issue is still present in version 0.203. The remote database is PostgreSQL 9.6 so I am using the PostgreSQL connector.

Using presto-cli to run a prepare statement where the query contains a reserved word as a table name returns a parsing error. Running the same statement with another character added onto the table name (even if it isn't a valid table in the remote database) so it is no longer a reserved word, works.

For example:
PREPARE stmt0 FROM SELECT id FROM "public"."case";
returns

Formatted query does not parse: Query{queryBody=QuerySpecification{select=Select{distinct=false, selectItems=[id]}, from=Optional[Table{public.case}], where=null, groupBy=Optional.empty, having=null, orderBy=Optional.empty, limit=null}, orderBy=Optional.empty}

Adding another character to the table name does not return the error:
PREPARE stmt0 FROM SELECT id FROM "public"."case2";
returns
PREPARE.

@ankitdixit
Copy link
Contributor

ankitdixit commented Jan 14, 2019

@findepi It seems that the output of formatter forgets double quotes and the parser fails when it tries to parse the formatted sql.
Would it make sense to add double quotes to formatted names anyway by removing if case here:
`private static String formatName(String name)
{

        if (NAME_PATTERN.matcher(name).matches()) {
            return name;
        }
        return "\"" + name.replace("\"", "\"\"") + "\"";
    }`

This would have a side effect that in show create table the names would come as double quoted even if they do not overlap with keywords.
Please let me know if there is a better way to find whether the name being formatted is a keyword.

@Praveen2112
Copy link
Contributor

Praveen2112 commented Jan 14, 2019

One solution is to capture the quoted identifier in the QualifiedName and use to re-create the same name when we are using SqlFormatter. Identfier has an API isDelimited which tells us whether the query has the table name as order or "ORDER". If we could fetch that info and push it to the QualifiedObjectName then we might modify the SqlFormatter#formatName as

        private static String formatName(QualifiedName name)
        {
            StringJoiner joiner = new StringJoiner(".");
            for (int part = 0; part < name.getOriginalParts().size(); part++) {
                String formattedName = name.getOriginalParts().get(part).replace("\"", "\"\"");
                if (name.getIsDelimited().get(part)) {
                    formattedName = "\"" + formattedName + "\"";
                }
                joiner.add(formattedName);
            }
            return joiner.toString();
        }

@findepi Your insights on this ?

@jeff303
Copy link

jeff303 commented May 20, 2021

It's not just reserved words, but it would seem if any component contain a dash, this same error occurs. Sample class (using presto-jdbc-0.254.jar driver, running against server version 0.254-fbd83ef):

import java.util.Properties;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.Statement;
import java.sql.DriverManager;
import java.sql.ResultSet;

public class TestQuoting {
  public static void main(String[] args) throws Exception {
    String url = "jdbc:presto://host:port/my_catalog/";
    Properties properties = new Properties();
    properties.setProperty("user", "myuser");
    properties.setProperty("password", "");
    properties.setProperty("SSL", "true");
    try (Connection connection = DriverManager.getConnection(url, properties);
         PreparedStatement stmt = connection.prepareStatement("DROP TABLE IF EXISTS \"my_catalog\".\"my_schema\".\"my_table\"");) {
      stmt.executeUpdate();
    }
  }
}

Simply changing my_schema to my-schema in that String will cause the com.facebook.presto.spi.PrestoException mentioned above.

@findepi
Copy link
Contributor

findepi commented May 21, 2021

@jeff303 Presto 3xx releases (now called Trino) are maintained at https://github.com/trinodb/trino.
Also, there is the #troubleshooting channel on Trino Community Slack (https://trino.io/slack.html)

@jeff303
Copy link

jeff303 commented May 21, 2021

@jeff303 Presto 3xx releases (now called Trino) are maintained at https://github.com/trinodb/trino.
Also, there is the #troubleshooting channel on Trino Community Slack (https://trino.io/slack.html)

Thanks for the pointer. Unfortunately, in this particular case, I need to work with a cluster using the Presto client protocol, as opposed to the Trino client protocol (which are not compatible as per here). I'll find a way to work around this issue.

jeff303 added a commit to metabase/presto-mb-ci that referenced this issue Jun 1, 2021
Updating Presto instance to have SSL enabled (in addition to HTTP/insecure)

Adding versions of every catalog that have "-" replaced with "_" due to a bug
in the Presto JDBC driver (prestodb/presto#10739)

Exposing SSL port from Docker image

Adding convenient build script
v-jizhang added a commit to v-jizhang/presto that referenced this issue Feb 22, 2022
Cherry-pick of trinodb/trino#80

Presto doesn't maintain the quotedness of an identifier when
using SqlQueryFormatter so it results in throwing parsing error
when we run prepare query of the syntax
[prestodb#10739].
This PR solves that above issue.

Co-authored-by: praveenkrishna <praveenkrishna@tutanota.com>
zhenxiao pushed a commit that referenced this issue Feb 24, 2022
Cherry-pick of trinodb/trino#80

Presto doesn't maintain the quotedness of an identifier when
using SqlQueryFormatter so it results in throwing parsing error
when we run prepare query of the syntax
[#10739].
This PR solves that above issue.

Co-authored-by: praveenkrishna <praveenkrishna@tutanota.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants