gortiz commented on code in PR #13943:
URL: https://github.com/apache/pinot/pull/13943#discussion_r1801076744
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java:
##########
@@ -100,26 +111,64 @@ public class QueryEnvironment {
private final FrameworkConfig _config;
private final CalciteCatalogReader _catalogReader;
private final HepProgram _optProgram;
- private final HepProgram _traitProgram;
-
- // Pinot extensions
- private final TableCache _tableCache;
- private final WorkerManager _workerManager;
+ private final Config _envConfig;
- public QueryEnvironment(String database, TableCache tableCache, @Nullable
WorkerManager workerManager) {
- PinotCatalog catalog = new PinotCatalog(tableCache, database);
+ public QueryEnvironment(Config config) {
+ _envConfig = config;
+ String database = config.getDatabase();
+ PinotCatalog catalog = new PinotCatalog(config.getTableCache(), database);
CalciteSchema rootSchema = CalciteSchema.createRootSchema(false, false,
database, catalog);
_config =
Frameworks.newConfigBuilder().traitDefs().operatorTable(PinotOperatorTable.instance())
.defaultSchema(rootSchema.plus()).sqlToRelConverterConfig(PinotRuleUtils.PINOT_SQL_TO_REL_CONFIG).build();
_catalogReader = new CalciteCatalogReader(rootSchema, List.of(database),
_typeFactory, CONNECTION_CONFIG);
_optProgram = getOptProgram();
- _traitProgram = getTraitProgram();
- _tableCache = tableCache;
- _workerManager = workerManager;
}
- private PlannerContext getPlannerContext() {
- return new PlannerContext(_config, _catalogReader, _typeFactory,
_optProgram, _traitProgram);
+ public QueryEnvironment(String database, TableCache tableCache, @Nullable
WorkerManager workerManager) {
+ this(configBuilder()
+ .database(database)
+ .tableCache(tableCache)
+ .workerManager(workerManager)
+ .build());
+ }
+
+ /**
+ * Returns a planner context that can be used to either parse, explain or
execute a query.
+ */
+ private PlannerContext getPlannerContext(SqlNodeAndOptions
sqlNodeAndOptions) {
+ WorkerManager workerManager = getWorkerManager(sqlNodeAndOptions);
+ HepProgram traitProgram = getTraitProgram(workerManager);
+ return new PlannerContext(_config, _catalogReader, _typeFactory,
_optProgram, traitProgram);
+ }
+
+ @Nullable
+ private WorkerManager getWorkerManager(SqlNodeAndOptions sqlNodeAndOptions) {
+ String useImplicitColocatedOptionValue = sqlNodeAndOptions.getOptions()
+
.get(CommonConstants.Broker.Request.QueryOptionKey.IMPLICIT_COLOCATE_JOIN);
+ WorkerManager workerManager = _envConfig.getWorkerManager();
+
+ if (useImplicitColocatedOptionValue == null) {
+ return _envConfig.useImplicitColocatedByDefault() ? workerManager : null;
+ }
+ switch (useImplicitColocatedOptionValue.toLowerCase()) {
+ case "true":
+ Objects.requireNonNull(workerManager, "WorkerManager is required for
implicit colocated join");
Review Comment:
Now that worker manager is enabled by default is more difficult, but if
`pinot.broker.enable.partition.metadata.manager` is set to false and then the
query includes `set implicitColocateJoin=true;`, then we fail this check.
It is just a matter of failing fast. The writer asked to use this feature
and it cannot be used because config indicates that worker manager should not
be instantiated. We can just ignore the writer request, but given this is not a
hint but an option, I think it is more elegant to fail with a correct message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
