feat(sqllogictest): Add sqllogictest schedule definition and parsing#1630
feat(sqllogictest): Add sqllogictest schedule definition and parsing#1630liurenjie1024 merged 7 commits intoapache:mainfrom
Conversation
liurenjie1024
left a comment
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @lliangyu-lin for this pr, LGTM! Just one nit.
| Ok(Self::new(engines, steps)) | ||
| } | ||
|
|
||
| pub async fn run(mut self) -> anyhow::Result<()> { |
There was a problem hiding this comment.
Add a log about current schedule file?
crates/sqllogictest/src/schedule.rs
Outdated
| .ok_or_else(|| anyhow!("Engine {} not found", step.engine))?; | ||
|
|
||
| engine | ||
| .run_slt_file(&PathBuf::from(step.slt.clone())) |
There was a problem hiding this comment.
This maybe a potential issue, the file path is relative to testdata dir, but we could fix it later.
There was a problem hiding this comment.
Good point, added the relative path in the previous draft PR, but missed it here. https://github.com/apache/iceberg-rust/pull/1621/files#diff-26b82f60b13d2afef45d856281a6fae47fefd48a440613ddcdd27059aec7639bR145-R150
|
Build seems to be broken. Put out a fix for it. Need to rerun after #1635 |
| } | ||
|
|
||
| impl DataFusionEngine { | ||
| pub async fn new(config: TomlTable) -> Result<Self> { |
There was a problem hiding this comment.
Why we move this out of trait?
liurenjie1024
left a comment
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @lliangyu-lin for this pr!
2 participants
Which issue does this PR close?
What changes are included in this PR?
Are these changes tested?
Yes, through unit tests. Some parts, like parse engines, require integrations and will be tested once engine and catalog support is added as part of the sql logic tests.