-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Improve Expanding Lookup Join performance by pushing a filter to the lookup join #132076
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
Conversation
🔍 Preview links for changed docs |
dc1e1a2
to
ee11e41
Compare
); | ||
builder.append( | ||
new FieldAttribute(Source.EMPTY, "Positions", new EsField("Positions", DataType.INTEGER, Collections.emptyMap(), false)) | ||
); |
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.
@nik9000
FilterOperator.FilterOperatorFactory needs an ExpressionEvaluator.Factory which needs a Layout.
How do I build a Layout here?
I attached 2 columns from the EnrichQuerySourceOperator and then whatever else we have in request.extractFields. It seems to work, because we don't refer to the first 2 columns.
EnrichQuerySourceOperator says there are 2 columns so I added them, but not sure what I have is correct. In particular the Docs column should be a vector, right? But there is no vector datatype.
} | ||
|
||
var evaluatorFactory = EvalMapper.toEvaluator( | ||
FoldContext.small()/*is this correct*/, |
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.
@nik9000 Should I use FoldContext.small() here?
ee11e41
to
673183a
Compare
releasables.add(extractFieldsOperator); | ||
operators.add(extractFieldsOperator); | ||
} | ||
if (queryList instanceof PostJoinFilterable postJoinFilterable) { | ||
FilterExec filterExec = postJoinFilterable.getPostJoinFilter(); |
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.
Why not update the logical plan (within the fragment) with a Filter
atop the source for the lookup index and let the existing infra generate the operator?
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.
@bpintea I am not sure I completely sure I understand, can you please expand? Do you mean generate it on the data node? We don't do logical or any other planning on the lookup node right now. This will be executed on the node that contains the lookup index. It filters the data before it is sent to the data node. So I think it needs to use the shard context and such from the lookup index node.
Rebased the PR, to be merged with #132889 |
No description provided.