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

Table selector won't work if table name is null #8842

Closed
LaroLuo opened this issue Jun 6, 2022 · 6 comments
Closed

Table selector won't work if table name is null #8842

LaroLuo opened this issue Jun 6, 2022 · 6 comments
Labels
beginner-task Small task for new contributors to ramp up help wanted

Comments

@LaroLuo
Copy link

LaroLuo commented Jun 6, 2022

* Executes a query.
*
* @param query The query to execute
* @return The result of the query
* @throws PinotClientException If an exception occurs while processing the query
*/
public ResultSetGroup execute(String query) {
return execute(null, query);
}

@Jackie-Jiang
Copy link
Contributor

Thanks for reporting the issue.

If table name is not provided, we should parse the query and extract the table name from the query. This requires pulling in the CalciteSqlParser class, which is currently in pinot-common module.
We can either:

  • Make pinot-client depend on pinot-common
  • Pull CalciteSqlParser into pinot-spi

cc @xiangfu0

@npawar npawar added help wanted beginner-task Small task for new contributors to ramp up labels Jun 13, 2022
@npawar
Copy link
Contributor

npawar commented Jun 13, 2022

Assuming we are okay with these dependencies in pinot-client @xiangfu0 , labeling this as beginner as it looks straightforward enough to fix. Lmk if it is not.

@tanmesh
Copy link
Contributor

tanmesh commented Aug 21, 2022

I am planning to pick this up making the pinot-client depend on pinot-common.

@tanmesh
Copy link
Contributor

tanmesh commented Aug 22, 2022

Hey @npawar @Jackie-Jiang 👋 , can you please assign someone to review this PR 🙏

@xiangfu0
Copy link
Contributor

The PR is on the right track.
One thing worth mentioning is that the get table name is the best effort.
And if the tableName is null, the brokerSelector should return any broker. E.g. select 1 is usually used for testing the db connection by many external systems. This kind of query has no table name, but also a valid query.

@Jackie-Jiang
Copy link
Contributor

Solved with #9902

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner-task Small task for new contributors to ramp up help wanted
Projects
None yet
Development

No branches or pull requests

5 participants