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

[Pulsar SQL] Make the Pulsar SQL support query the uppercase topic #9980

Merged
merged 6 commits into from
Mar 24, 2021

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Mar 19, 2021

Motivation

When querying the uppercase topic (e.g. public/default/case_UPPER_topic) by the Pulsar SQL, it will throw the error "topic not found" because of the class SchemaTableName, in the Pulsar SQL, we couldn't get the exact table name (e.g. Test, tEst) but only lowercase table name (e.g. test), this is the presto behavior.

Refer to the presto class SchemaTableName.

@JsonCreator
public SchemaTableName(@JsonProperty("schema") String schemaName, @JsonProperty("table") String tableName) {
    this.schemaName = SchemaUtil.checkNotEmpty(schemaName, "schemaName").toLowerCase(Locale.ENGLISH);
    this.tableName = SchemaUtil.checkNotEmpty(tableName, "tableName").toLowerCase(Locale.ENGLISH);
}

If there are topics public/default/case_UPPER_topic, public/default/case_upper_topic, the query show tables in pulsar."public/default" will return one result public/default/case_upper_topic, this behavior is determined by the presto spi.

There is a precondition for querying the uppercase topic, the topic name must be unique. For example, if there are topics public/default/case_UPPER_topic, public/default/case_upper_topic, the query select * from pulsar."public/default"."case_upper_topic" will throw error no matched topic, because of that we could get the exact topic by the lowercase table name case_upper_topic`.

If there is a unique topic public/default/case_UPPER_topic, the query select * from pulsar."public/default"."case_UPPER_topic or select * from pulsar."public/default"."case_upper_topic will query that unique topic.

topics allow query table name
test, Test false -
test true test, Test

Modifications

Get the matched topic by the lowercase table name in the method getTableHandle.

Verifying this change

Add new integration tests.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

private static final Logger log = Logger.get(PulsarMetadata.class);

private final LoadingCache<SchemaTableName, TopicName> tableNameTopicNameCache =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the static is necessary, this class is a singleton class, and if changing this field to static, the corresponding methods and field should be changed to static.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui
Copy link
Contributor

Remove the release/2.7.2 tag since this is not a serious problem and branch-2.7 does not contain PIP-71, there are lots of conflicts when cherry-picking this one.

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

Successfully merging this pull request may close these issues.

None yet

3 participants