-
Notifications
You must be signed in to change notification settings - Fork 590
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
test: add duckdb cte tests & fix some minor bugs #12527
Conversation
Makefile.toml
Outdated
"--help", | ||
], install_command = "binstall" } | ||
command = "sqllogictest" | ||
args = ["${@}"] | ||
env = { SLT_PORT = "4566", SLT_DB = "dev" } |
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.
So that risedev slt
doesn't need to add -p 4566 -d dev
any more. (If flags are present, they should have higher precedence)
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.
What about leveraging the risedev_env
way?
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?
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.
Hard-coded 4566 is not always the listen port. 🤔
🤔️ |
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.
Rest LGTM
Makefile.toml
Outdated
"--help", | ||
], install_command = "binstall" } | ||
command = "sqllogictest" | ||
args = ["${@}"] | ||
env = { SLT_PORT = "4566", SLT_DB = "dev" } |
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.
What about leveraging the risedev_env
way?
Codecov Report
@@ Coverage Diff @@
## main #12527 +/- ##
==========================================
+ Coverage 69.43% 69.45% +0.01%
==========================================
Files 1438 1438
Lines 238698 238700 +2
==========================================
+ Hits 165740 165787 +47
+ Misses 72958 72913 -45
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
LGTM
# A bug in https://github.com/risingwavelabs/risingwave/pull/12527 | ||
# lookup join panics when the scan is single distribution | ||
query I | ||
select 1 from (select * from t1 left join t3 on t1.v1 = t3.v1 order by t1.v1, t1.v2) where exists (select 1) limit 1; | ||
---- | ||
1 |
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.
Could you please elaborate more about this case about lookup join panic?
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.
Basically, I used self.clone()
in two_phase_limit
instead of self.clone_with_input
, so its input is the one before to_distributed
is called: LocalLookupJoin
+ BatchScan
(Single dist).
What's interesting is that without where exists (select 1)
the result is correct.
dev=> explain (verbose) select 1 from (select * from t1 left join t3 on t1.v1 = t3.v1 order by t1.v1, t1.v2) where exists (select 1) limit 1;
QUERY PLAN
-----------------------------------------------------------------------------------
BatchExchange { order: [], dist: Single }
└─BatchProject { exprs: [1:Int32] }
└─BatchLimit { limit: 1, offset: 0 }
└─BatchNestedLoopJoin { type: LeftSemi, predicate: true, output: all }
├─BatchLookupJoin { type: LeftOuter, predicate: t1.v1 = t3.v1, output: [] }
│ └─BatchScan { table: t1, columns: [t1.v1], distribution: Single }
└─BatchValues { rows: [[]] }
(7 rows)
dev=>
dev=> explain (verbose) select 1 from (select * from t1 left join t3 on t1.v1 = t3.v1 order by t1.v1, t1.v2) limit 1;
QUERY PLAN
-----------------------------------------------------------------------------------
BatchProject { exprs: [1:Int32] }
└─BatchLimit { limit: 1, offset: 0 }
└─BatchExchange { order: [], dist: Single }
└─BatchLimit { limit: 1, offset: 0 }
└─BatchLookupJoin { type: LeftOuter, predicate: t1.v1 = t3.v1, output: [] }
└─BatchExchange { order: [], dist: UpstreamHashShard(t1.v1) }
└─BatchScan { table: t1, columns: [t1.v1], distribution: SomeShard }
(7 rows)
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
#5987
fix #12511
fix #12526
not fixed yet: #12513
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.