Skip to content

Sql shell warnings#9237

Merged
NathanGabrielson merged 22 commits intomainfrom
nathan/sql-shell-warnings
May 22, 2025
Merged

Sql shell warnings#9237
NathanGabrielson merged 22 commits intomainfrom
nathan/sql-shell-warnings

Conversation

@NathanGabrielson
Copy link
Copy Markdown
Contributor

Fix issue #8875

This pr adds support for warnings in the sql shell. There is now both a summary and detailed list of warnings.

Running select 1/0; will produce, after the table:

Warning (Code 1365): Division by 0```

You can disable/enable the ending list with \w and \W, respectively.

@NathanGabrielson
Copy link
Copy Markdown
Contributor Author

NathanGabrielson commented May 21, 2025

The only thing I'm a little unsure about is that the new code blows up the log file --> you basically run 7 extra prompts every command, which all throw 3-4 lines in the log so it's really hard to tell what's going on. I think it would be possible, but really tedious to debug what's actually going on with the -l debug flag. Might want to clean this up first.

Some notes for tmrw for myself:

  1. Show warnings (and maybe some other commands) dump a bunch of newline characters
  2. Figure out why some tests failed

@NathanGabrielson NathanGabrielson marked this pull request as draft May 21, 2025 00:18
@coffeegoddd
Copy link
Copy Markdown
Contributor

@NathanGabrielson DOLT

comparing_percentages
100.000000 to 100.000000
version result total
38156e2 ok 5937457
version total_tests
38156e2 5937457
correctness_percentage
100.0

@NathanGabrielson NathanGabrielson marked this pull request as ready for review May 22, 2025 17:12
@NathanGabrielson NathanGabrielson changed the title Nathan/sql shell warnings Sql shell warnings May 22, 2025
@coffeegoddd
Copy link
Copy Markdown
Contributor

@NathanGabrielson DOLT

comparing_percentages
100.000000 to 100.000000
version result total
21f9b40 ok 5937457
version total_tests
21f9b40 5937457
correctness_percentage
100.0

Copy link
Copy Markdown
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were enough requests for changes that I'd like to take another look before you ship it. All simple stuff - the flimsy show warnings equality check needs a fix and testing.

@NathanGabrielson
Copy link
Copy Markdown
Contributor Author

The only thing I'm a little unsure about is the show warnings check string manipulation? I had to do a bunch of stuff and was wondering if you think a helper function would be better? Is there a nicer/more conventional way to remove the whitespace?

@coffeegoddd
Copy link
Copy Markdown
Contributor

@NathanGabrielson DOLT

comparing_percentages
100.000000 to 100.000000
version result total
98d9763 ok 5937457
version total_tests
98d9763 5937457
correctness_percentage
100.0

Copy link
Copy Markdown
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just add a test to verify that SHoW WarnINGS behaves.

@coffeegoddd
Copy link
Copy Markdown
Contributor

@NathanGabrielson DOLT

comparing_percentages
100.000000 to 100.000000
version result total
8fe1eb9 ok 5937457
version total_tests
8fe1eb9 5937457
correctness_percentage
100.0

@NathanGabrielson NathanGabrielson merged commit b3cdeaa into main May 22, 2025
21 checks passed
@github-actions
Copy link
Copy Markdown

@coffeegoddd DOLT

test_name detail row_cnt sorted mysql_time sql_mult cli_mult
batching LOAD DATA 10000 1 0.07 1.29
batching batch sql 10000 1 0.1 1.2
batching by line sql 10000 1 0.1 1.3
blob 1 blob 200000 1 0.91 3.85 4.47
blob 2 blobs 200000 1 0.88 4.5 4.77
blob no blob 200000 1 1.11 2.08 2.38
col type datetime 200000 1 0.88 2.31 2.67
col type varchar 200000 1 0.74 3.58 3.76
config width 2 cols 200000 1 0.95 2.17 2.44
config width 32 cols 200000 1 1.97 1.94 2.7
config width 8 cols 200000 1 0.99 2.38 2.72
pk type float 200000 1 0.9 2.31 3.18
pk type int 200000 1 0.8 2.63 2.95
pk type varchar 200000 1 2 1.32 1.41
row count 1.6mm 1600000 1 5.84 2.95 2.99
row count 400k 400000 1 1.53 2.76 2.98
row count 800k 800000 1 2.99 2.87 2.9
secondary index four index 200000 1 3.75 1.37 1.28
secondary index no secondary 200000 1 1.06 2.11 2.52
secondary index one index 200000 1 1.18 2.39 2.52
secondary index two index 200000 1 2.12 1.68 1.8
sorting shuffled 1mm 1000000 0 5.37 2.86 2.74
sorting sorted 1mm 1000000 1 5.34 2.81 2.77

@github-actions
Copy link
Copy Markdown

@coffeegoddd DOLT

name detail mean_mult
dolt_blame_basic system table 1.19
dolt_blame_commit_filter system table 2.86
dolt_commit_ancestors_commit_filter system table 0.63
dolt_commits_commit_filter system table 1.1
dolt_diff_log_join_from_commit system table 2.76
dolt_diff_log_join_to_commit system table 2.75
dolt_diff_table_from_commit_filter system table 1.17
dolt_diff_table_to_commit_filter system table 1.17
dolt_diffs_commit_filter system table 1.03
dolt_history_commit_filter system table 1.5
dolt_log_commit_filter system table 1.16

@github-actions
Copy link
Copy Markdown

@coffeegoddd DOLT

name add_cnt delete_cnt update_cnt latency
adds_only 60000 0 0 1.14
adds_updates_deletes 60000 60000 60000 4.44
deletes_only 0 60000 0 2.38
updates_only 0 0 60000 3.01

@tbantle22 tbantle22 deleted the nathan/sql-shell-warnings branch May 28, 2025 21:35
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