-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(tesseract): Pre-aggregation planning fallback #9230
base: tesseract-exclude-values-from-filter-params
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -278,11 +278,9 @@ export class BaseQuery { | |||||
return dimension; | ||||||
}).filter(R.identity).map(this.newTimeDimension.bind(this)); | ||||||
this.allFilters = this.timeDimensions.concat(this.segments).concat(this.filters); | ||||||
this.useNativeSqlPlanner = this.options.useNativeSqlPlanner ?? getEnv('nativeSqlPlanner'); | ||||||
this.prebuildJoin(); | ||||||
|
||||||
if (!getEnv('nativeSqlPlanner')) { | ||||||
// Tesseract doesn't require join to be prebuilt and there's a case where single join can't be built for multi-fact query | ||||||
this.join = this.joinGraph.buildJoin(this.allJoinHints); | ||||||
} | ||||||
this.cubeAliasPrefix = this.options.cubeAliasPrefix; | ||||||
this.preAggregationsSchemaOption = this.options.preAggregationsSchema ?? DEFAULT_PREAGGREGATIONS_SCHEMA; | ||||||
this.externalQueryClass = this.options.externalQueryClass; | ||||||
|
@@ -299,6 +297,15 @@ export class BaseQuery { | |||||
this.initUngrouped(); | ||||||
} | ||||||
|
||||||
prebuildJoin() { | ||||||
/* if (!this.useNativeSqlPlanner) { We still need this join for the follback to preaggregation to work properly. This condition should be returned after the tesseract starts working with pre-aggregations | ||||||
// Tesseract doesn't require join to be prebuilt and there's a case where single join can't be built for multi-fact query | ||||||
this.join = this.joinGraph.buildJoin(this.allJoinHints); | ||||||
} */ | ||||||
|
||||||
this.join = this.joinGraph.buildJoin(this.allJoinHints); | ||||||
} | ||||||
|
||||||
cacheValue(key, fn, { contextPropNames, inputProps, cache } = {}) { | ||||||
const currentContext = this.safeEvaluateSymbolContext(); | ||||||
if (contextPropNames) { | ||||||
|
@@ -375,8 +382,7 @@ export class BaseQuery { | |||||
initUngrouped() { | ||||||
this.ungrouped = this.options.ungrouped; | ||||||
if (this.ungrouped) { | ||||||
// this.join is not defined for Tesseract | ||||||
if (!this.options.allowUngroupedWithoutPrimaryKey && !getEnv('nativeSqlPlanner')) { | ||||||
if (!this.options.allowUngroupedWithoutPrimaryKey) { | ||||||
const cubes = R.uniq([this.join.root].concat(this.join.joins.map(j => j.originalTo))); | ||||||
const primaryKeyNames = cubes.flatMap(c => this.primaryKeyNames(c)); | ||||||
const missingPrimaryKeys = primaryKeyNames.filter(key => !this.dimensions.find(d => d.dimension === key)); | ||||||
|
@@ -603,33 +609,61 @@ export class BaseQuery { | |||||
return false; | ||||||
} | ||||||
|
||||||
newQueryNotUseNative() { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
const QueryClass = this.constructor; | ||||||
return new QueryClass(this.compilers, { ...this.options, useNativeSqlPlanner: false }); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns a pair of SQL query string and parameter values for the query. | ||||||
* @param {boolean} [exportAnnotatedSql] - returns annotated sql with not rendered params if true | ||||||
* @returns {[string, Array<unknown>]} | ||||||
*/ | ||||||
buildSqlAndParams(exportAnnotatedSql) { | ||||||
if (getEnv('nativeSqlPlanner')) { | ||||||
return this.buildSqlAndParamsRust(exportAnnotatedSql); | ||||||
} else { | ||||||
if (!this.options.preAggregationQuery && !this.options.disableExternalPreAggregations && this.externalQueryClass) { | ||||||
if (this.externalPreAggregationQuery()) { // TODO performance | ||||||
return this.externalQuery().buildSqlAndParams(exportAnnotatedSql); | ||||||
if (!this.options.preAggregationQuery && !this.options.disableExternalPreAggregations && this.externalQueryClass) { | ||||||
if (this.externalPreAggregationQuery()) { // TODO performance | ||||||
return this.externalQuery().buildSqlAndParams(exportAnnotatedSql); | ||||||
} | ||||||
} | ||||||
|
||||||
if (this.useNativeSqlPlanner) { | ||||||
let isRelatedToPreAggregation = false; | ||||||
if (this.options.preAggregationQuery) { | ||||||
isRelatedToPreAggregation = true; | ||||||
} else if (!this.options.disableExternalPreAggregations && this.externalQueryClass) { | ||||||
if (this.externalPreAggregationQuery()) { | ||||||
isRelatedToPreAggregation = true; | ||||||
} | ||||||
} else { | ||||||
let preAggForQuery = | ||||||
this.preAggregations.findPreAggregationForQuery(); | ||||||
if (this.options.disableExternalPreAggregations && preAggForQuery && preAggForQuery.preAggregation.external) { | ||||||
preAggForQuery = undefined; | ||||||
} | ||||||
if (preAggForQuery) { | ||||||
isRelatedToPreAggregation = true; | ||||||
Comment on lines
+640
to
+644
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like these 2 ifs can be combined into one. Smth like: if (!(this.options.disableExternalPreAggregations &&
preAggForQuery &&
preAggForQuery.preAggregation.external)) {
isRelatedToPreAggregation = true; |
||||||
} | ||||||
} | ||||||
return this.compilers.compiler.withQuery( | ||||||
this, | ||||||
() => this.cacheValue( | ||||||
['buildSqlAndParams', exportAnnotatedSql], | ||||||
() => this.paramAllocator.buildSqlAndParams( | ||||||
this.buildParamAnnotatedSql(), | ||||||
exportAnnotatedSql, | ||||||
this.shouldReuseParams | ||||||
), | ||||||
{ cache: this.queryCache } | ||||||
) | ||||||
); | ||||||
|
||||||
if (isRelatedToPreAggregation) { | ||||||
return this.newQueryNotUseNative().buildSqlAndParams(exportAnnotatedSql); | ||||||
} | ||||||
|
||||||
return this.buildSqlAndParamsRust(exportAnnotatedSql); | ||||||
} | ||||||
|
||||||
return this.compilers.compiler.withQuery( | ||||||
this, | ||||||
() => this.cacheValue( | ||||||
['buildSqlAndParams', exportAnnotatedSql], | ||||||
() => this.paramAllocator.buildSqlAndParams( | ||||||
this.buildParamAnnotatedSql(), | ||||||
exportAnnotatedSql, | ||||||
this.shouldReuseParams | ||||||
), | ||||||
{ cache: this.queryCache } | ||||||
) | ||||||
); | ||||||
} | ||||||
|
||||||
buildSqlAndParamsRust(exportAnnotatedSql) { | ||||||
|
@@ -2960,6 +2994,7 @@ export class BaseQuery { | |||||
} | ||||||
|
||||||
newSubQueryForCube(cube, options) { | ||||||
options = { ...options, useNativeSqlPlanner: false }; // We not use tesseract for pre-aggregations generation yet | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
if (this.options.queryFactory) { | ||||||
// When dealing with rollup joins, it's crucial to use the correct parameter allocator for the specific cube in use. | ||||||
// By default, we'll use BaseQuery, but it's important to note that different databases (Oracle, PostgreSQL, MySQL, Druid, etc.) | ||||||
|
@@ -2983,6 +3018,7 @@ export class BaseQuery { | |||||
historyQueries: this.options.historyQueries, | ||||||
externalQueryClass: this.options.externalQueryClass, | ||||||
queryFactory: this.options.queryFactory, | ||||||
useNativeSqlPlanner: this.options.useNativeSqlPlanner, | ||||||
...options, | ||||||
}; | ||||||
} | ||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
It would be good to add
.default('false')
to this env so the useNativeSqlPlanner can be always bool.