-
Notifications
You must be signed in to change notification settings - Fork 184
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!: allow ProjectionExec
to take general inputs
#384
base: main
Are you sure you want to change the base?
Conversation
Replacing #377 since GitHub was stuck |
cc62e5e
to
668ace0
Compare
@@ -71,15 +87,15 @@ impl ProofPlan for ProjectionExec { | |||
} | |||
|
|||
fn get_column_references(&self) -> IndexSet<ColumnRef> { | |||
let mut columns = IndexSet::default(); | |||
let mut columns = self.input.get_column_references(); |
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.
let mut columns = self.input.get_column_references(); | |
return self.input.get_column_references(); |
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.
You are right. Projection should result in all output column refs also being input ones.
let t = "PLACEHOLDER_SCHEMA.PLACEHOLDER_TABLE".parse().unwrap(); | ||
let mut accessor = OwnedTableTestAccessor::<InnerProductProof>::new_empty_with_setup(()); | ||
accessor.add_table(t, data, 0); | ||
let ast = projection(cols_expr_plan(t, &["b"], &accessor), tab(t)); | ||
let ast = projection( | ||
cols_expr_plan(t, &["b"], &accessor), | ||
table_exec( | ||
t, | ||
vec![ | ||
ColumnField::new("a".parse().unwrap(), ColumnType::BigInt), | ||
ColumnField::new("b".parse().unwrap(), ColumnType::BigInt), | ||
], | ||
), | ||
); |
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.
let t = "PLACEHOLDER_SCHEMA.PLACEHOLDER_TABLE".parse().unwrap(); | |
let mut accessor = OwnedTableTestAccessor::<InnerProductProof>::new_empty_with_setup(()); | |
accessor.add_table(t, data, 0); | |
let ast = projection(cols_expr_plan(t, &["b"], &accessor), tab(t)); | |
let ast = projection( | |
cols_expr_plan(t, &["b"], &accessor), | |
table_exec( | |
t, | |
vec![ | |
ColumnField::new("a".parse().unwrap(), ColumnType::BigInt), | |
ColumnField::new("b".parse().unwrap(), ColumnType::BigInt), | |
], | |
), | |
); | |
let t = "sxt.t".parse().unwrap(); | |
let accessor = OwnedTableTestAccessor::<InnerProductProof>::new_from_table(t, data, 0, ()); | |
let ast = projection( | |
vec![AliasedDynProofExpr { | |
expr: DynProofExpr::Column(ColumnExpr::new(ColumnRef::new( | |
"PLACEHOLDER_SCHEMA.PLACEHOLDER_TABLE".parse().unwrap(), | |
"b".parse().unwrap(), | |
ColumnType::BigInt, | |
))), | |
alias: "b".parse().unwrap(), | |
}], | |
table_exec( | |
t, | |
vec![ | |
ColumnField::new("a".parse().unwrap(), ColumnType::BigInt), | |
ColumnField::new("b".parse().unwrap(), ColumnType::BigInt), | |
], | |
), | |
); |
…w` to line up with other `ProofPlan`s and `ProofExpr`s
Please be sure to look over the pull request guidelines here: https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md#submit-pr.
Please go through the following checklist
!
is used if and only if at least one breaking change has been introduced.source scripts/run_ci_checks.sh
.Rationale for this change
We need to make it possible to compose
ProjectionExec
withProofPlan
s.What changes are included in this PR?
TableExpr
inProjectionExec
withBox<DynProofPlan>
Are these changes tested?
Yes.