Skip to content
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

chore: rename streaming/group_top_n/main1.slt to main.slt #13187

Merged
merged 8 commits into from
Nov 7, 2023

Conversation

stdrc
Copy link
Member

@stdrc stdrc commented Oct 31, 2023

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Previously I named it main1.slt because of a limitation of sqllogictest-rs, now it's fixed (risinglightdb/sqllogictest-rs#194, risinglightdb/sqllogictest-rs#198).

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
  • My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
  • All checks passed in ./risedev check (or alias, ./risedev c)
  • My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)
  • My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

  • My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

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.

Signed-off-by: Richard Chien <stdrc@outlook.com>
@stdrc stdrc requested a review from xxchan October 31, 2023 17:57
@stdrc stdrc changed the title chore: rename streaming/group_top_n/main.slt to main.slt chore: rename streaming/group_top_n/main1.slt to main.slt Nov 1, 2023
@xxchan
Copy link
Member

xxchan commented Nov 1, 2023

QueryError: Catalog error: database with name main_slt exists"

@stdrc
Copy link
Member Author

stdrc commented Nov 1, 2023

Seems the sqllogictest version is not bumped? @xxchan

@xxchan
Copy link
Member

xxchan commented Nov 1, 2023

sqllogictest-bin@0.17.0 \

@xxchan
Copy link
Member

xxchan commented Nov 1, 2023

Oh, maybe we need to upgrade for sim. It uses lib instead of bin.

@stdrc
Copy link
Member Author

stdrc commented Nov 1, 2023

thread '<unnamed>' panicked at /risingwave/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sqllogictest-0.17.0/src/runner.rs:1035:18:
create db failed: Error { kind: Db, cause: Some(DbError { severity: "ERROR", parsed_severity: None, code: SqlState(EXX000), message: "QueryError: Catalog error: database with name main_slt exists", detail: None, hint: None, position: None, where_: None, schema: None, table: None, column: None, datatype: None, constraint: None, file: None, line: None, routine: None }) }
stack backtrace:
   0: rust_begin_unwind
             at /rustc/249624b5043013d18c00f0401ca431c1a6baa8cd/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_fmt
             at /rustc/249624b5043013d18c00f0401ca431c1a6baa8cd/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/249624b5043013d18c00f0401ca431c1a6baa8cd/library/core/src/result.rs:1653:5
   3: core::result::Result<T,E>::expect
             at /rustc/249624b5043013d18c00f0401ca431c1a6baa8cd/library/core/src/result.rs:1034:23
   4: sqllogictest::runner::Runner<D,M>::run_parallel_async::{{closure}}
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/sqllogictest-0.17.0/src/runner.rs:1032:13
   5: risingwave_simulation::slt::run_parallel_slt_task::{{closure}}
             at ./src/tests/simulation/src/slt.rs:237:10
   6: risingwave_simulation::main::{{closure}}::{{closure}}::{{closure}}
             at ./src/tests/simulation/src/main.rs:246:51

It's using 0.17, weird.

@stdrc
Copy link
Member Author

stdrc commented Nov 1, 2023

Oh, maybe we need to upgrade for sim. It uses lib instead of bin.

Submitted a new PR for sqllogictest-rs: risinglightdb/sqllogictest-rs#198

Signed-off-by: Richard Chien <stdrc@outlook.com>
@stdrc stdrc force-pushed the rc/rename-main1-slt branch from f07c4d0 to 80369b1 Compare November 1, 2023 15:13
@stdrc stdrc requested a review from a team as a code owner November 1, 2023 15:13
@stdrc stdrc requested a review from xxchan November 1, 2023 15:14
ci/Dockerfile Show resolved Hide resolved
Signed-off-by: Richard Chien <stdrc@outlook.com>
@stdrc stdrc enabled auto-merge November 1, 2023 15:41
@BugenZhao
Copy link
Member

Why audit keeps failing? 👀

Signed-off-by: Richard Chien <stdrc@outlook.com>
@stdrc stdrc added this pull request to the merge queue Nov 7, 2023
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #13187 (a8996ce) into main (04999f0) will decrease coverage by 0.06%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main   #13187      +/-   ##
==========================================
- Coverage   68.07%   68.01%   -0.06%     
==========================================
  Files        1521     1522       +1     
  Lines      257858   257904      +46     
==========================================
- Hits       175546   175425     -121     
- Misses      82312    82479     +167     
Flag Coverage Δ
rust 68.01% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/frontend/src/optimizer/logical_optimization.rs 98.05% <100.00%> (+0.03%) ⬆️
...end/src/optimizer/plan_node/logical_over_window.rs 91.70% <100.00%> (+0.08%) ⬆️
src/frontend/src/optimizer/rule/mod.rs 100.00% <ø> (ø)
...ntend/src/optimizer/rule/over_window_merge_rule.rs 100.00% <100.00%> (ø)

... and 18 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

Merged via the queue into main with commit 4fdbade Nov 7, 2023
7 of 8 checks passed
@stdrc stdrc deleted the rc/rename-main1-slt branch November 7, 2023 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants