-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[multistage] support database in v2 #12591
[multistage] support database in v2 #12591
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12591 +/- ##
============================================
- Coverage 61.75% 61.55% -0.20%
+ Complexity 207 198 -9
============================================
Files 2436 2456 +20
Lines 133233 134388 +1155
Branches 20636 20800 +164
============================================
+ Hits 82274 82721 +447
- Misses 44911 45495 +584
- Partials 6048 6172 +124
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we need to support a query option of database
? I kind of prefer always keeping it explicit in the query by using <database>.<tableName>
if it is not the default database
...ker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
Show resolved
Hide resolved
pinot-query-planner/src/main/java/org/apache/pinot/query/catalog/PinotCatalog.java
Outdated
Show resolved
Hide resolved
Using the postgres terminology we are not defining databases but schemas. Databases are independent of each other and for example you cannot join tables in two different databases. AFAIK in MySQL database and schema is the same thing. As we can see in I would strongly recommend to change the nomenclature to do not use database in that case. The correct SQL term would be schema but given we already use that term we may use namespace instead. BTW, in any schema may contain other schemas. I don't know if we plan to support that as well right now, but even if we don't support that in a first implementation, we should let open that possibility for the future. |
pinot-query-planner/src/main/java/org/apache/pinot/query/catalog/PinotCatalog.java
Outdated
Show resolved
Hide resolved
I don't think that is mandatory right now. We can have a first version without that. AFAIR in Postgres this is done with an option (in their case, |
cc0082a
to
9d91b32
Compare
pinot-query-planner/src/main/java/org/apache/calcite/jdbc/CalciteSchemaBuilder.java
Outdated
Show resolved
Hide resolved
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
Outdated
Show resolved
Hide resolved
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
Outdated
Show resolved
Hide resolved
pinot-common/src/main/java/org/apache/pinot/common/utils/DatabaseUtils.java
Show resolved
Hide resolved
pinot-common/src/main/java/org/apache/pinot/common/utils/DatabaseUtils.java
Outdated
Show resolved
Hide resolved
...-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
Outdated
Show resolved
Hide resolved
pinot-query-planner/src/main/java/org/apache/pinot/query/catalog/PinotCatalog.java
Outdated
Show resolved
Hide resolved
…k219/pinot into db-support-in-v2-engine
...ery-planner/src/main/java/org/apache/pinot/query/planner/logical/RelToPlanNodeConverter.java
Show resolved
Hide resolved
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTest.java
Show resolved
Hide resolved
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
This PR allows multi stage engine to support the queries with database context
The database context can be passed using
database
http headerdatabase
query optiondatabase
http headerThe query request needs to have the http header
Database: db1
.User can query the tables under database
db1
withdatabase
query optionThe query option
database=db1
must either be passed as part of the request payload or through query as given belowdefault
database handlingUser can skip passing the database context in this case or can even pass
default
database context using anyone of the above mentioned options, both will have equivalent behaviour