Skipping codegen and compile for Scan only plan #3853
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Lantao Jin <ltjin@amazon.com>
LantaoJin
added
enhancement
LantaoJin
requested review from
GumpacG,
MaxKsyunz,
Swiddis,
YANG-DB,
Yury-Fridlyand,
acarbonetto,
anirudha,
dai-chen,
derek-ho,
forestmvey,
joshuali925,
kavithacm,
mengweieric,
noCharger,
penghuo,
ps48,
qianheng-aws,
seankao-az and
ykmr1224
as code owners
Signed-off-by: Lantao Jin <ltjin@amazon.com>
songkant-aws
reviewed
Jul 8, 2025
| import org.apache.calcite.linq4j.Enumerable; | ||
| import org.checkerframework.checker.nullness.qual.Nullable; | ||
|
|
||
| public interface Scannable { |
There was a problem hiding this comment.
Nit: Add javadoc to explain
| } | ||
|
|
||
| @Override | ||
| protected Function0<CalcitePrepare> createPrepareFactory() { |
There was a problem hiding this comment.
Nit: Looks like this method is marked as @Deprecated. Use withPrepareFactory() in OpenSearchDriver instead?
There was a problem hiding this comment.
Actually we cannot use withPrepareFactory since withPrepareFactory will return a Driver instead of OpenSearchDriver. We need the added connect() method of OpenSearchDriver.
There was a problem hiding this comment.
Em, maybe we could with casting.
try {
OpenSearchDriver driver = (OpenSearchDriver) new OpenSearchDriver().withPrepareFactory(OpenSearchPrepareImpl::new);
return driver.connect("jdbc:calcite:", info, null, typeFactory);
} catch (SQLException e) {
throw new RuntimeException(e);
}
Anyway, let' keep current createPrepareFactory() unless it is removed in future.
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
qianheng-aws
approved these changes
Jul 8, 2025
penghuo
reviewed
Jul 9, 2025
Comment on lines
+248
to
+256
| final JavaTypeFactory typeFactory = context.getTypeFactory(); | ||
| final EnumerableRel.Prefer prefer; | ||
| if (elementType == Object[].class) { | ||
| prefer = EnumerableRel.Prefer.ARRAY; | ||
| } else { | ||
| prefer = EnumerableRel.Prefer.CUSTOM; | ||
| } | ||
| final Convention resultConvention = | ||
| enableBindable ? BindableConvention.INSTANCE : EnumerableConvention.INSTANCE; |
There was a problem hiding this comment.
add a comments if copy from calcite? e.g.
// START FROM CALCITE
// END FROM CALCITE
| } | ||
|
|
||
| @Override | ||
| protected PreparedResult implement(RelRoot root) { |
There was a problem hiding this comment.
Can we leverage BindableConvention.INSTANCE and regsiter a rule for convertion?
There was a problem hiding this comment.
No, Bindable and Enumerable conventions cannot work together. The resultConvention of physical plan can only be one of them.
penghuo
approved these changes
Jul 10, 2025
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Jul 10, 2025
* Skipping codegen and compile for Scan only plan Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix explain IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix explain IT without pushdown Signed-off-by: Lantao Jin <ltjin@amazon.com> * add javadoc Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com> (cherry picked from commit a3cd42e) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
qianheng-aws
pushed a commit
that referenced
this pull request
Jul 11, 2025
…3866) * Skipping codegen and compile for Scan only plan (#3853) * Skipping codegen and compile for Scan only plan Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix explain IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix explain IT without pushdown Signed-off-by: Lantao Jin <ltjin@amazon.com> * add javadoc Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com> (cherry picked from commit a3cd42e) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Fix compile error in JDK11 Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Lantao Jin <ltjin@amazon.com>
7 tasks
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Currently, when the Calcite engine is enabled, all query plans -- regardless of their structure -- are converted to
EnumerableRel. This means the plan is transformed into aLinq4jExpression, generates Java code, and undergoes just-in-time (JIT) compilation before execution.For simple or fully pushdown-compatible queries (e.g.,
source=t,source=t | where a=1,source=t | sort a, orsource=t | where a=1 | sort b | head 10), the optimized plans only contain nodeEnumerableTableScan. Despite their simplicity, these plans unnecessarily undergo code generation and dynamic compilation, introducing overhead.To reduce codegen and compilation time, we propose converting such plans to
BindableRelinstead ofEnumerableRel. This bypasses code generation and compilation entirely, improving plan execution time by ~30% (By benchmarking the simple queries by #3822)Related Issues
Resolves #3852
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.