Skip to content

!! toby/arg_cleanup#6330

Merged
tobymao merged 4 commits intomainfrom
toby/arg_cleanup
Nov 15, 2025
Merged

!! toby/arg_cleanup#6330
tobymao merged 4 commits intomainfrom
toby/arg_cleanup

Conversation

@tobymao
Copy link
Owner

@tobymao tobymao commented Nov 15, 2025

No description provided.

@tobymao tobymao requested a review from georgesittas November 15, 2025 05:18
@tobymao tobymao changed the title Toby/arg cleanup !! toby/arg_cleanup Nov 15, 2025
if k not in self.arg_types:
errors.append(f"Unexpected keyword: '{k}' for {self.__class__}")
for k, mandatory in self.arg_types.items():
if UNITTEST:
Copy link
Owner Author

Choose a reason for hiding this comment

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

the check for unknown args only runs at parse time. so for 3rd party
libraries or other code like the optimizer setting args or creating
expressions themselves, it's never checked it or worked. moving this to
unit testing only.

@tobymao
Copy link
Owner Author

tobymao commented Nov 15, 2025

/benchmark

the check for unexpected args only runs at parse time. so for 3rd party
libraries or other code like the optimizer setting args or creating
expressions themselves, it's never checked it or worked. moving this to
unit testing only.
@tobymao
Copy link
Owner Author

tobymao commented Nov 15, 2025

/benchmark

@github-actions
Copy link
Contributor

Benchmark Results

Legend:

  • 🟢🟢 = 2x+ faster
  • 🟢 = 1.1x - 2x faster
  • ⚪ = No significant change (< 1.1x)
  • 🔴 = 1.1x - 2x slower
  • 🔴🔴 = 2x+ slower

Parsing Benchmark

Benchmark bench_parse_main bench_parse_pr
parse_sqlglotrs_short 298 us 296 us: ⚪ 1.01x faster
parse_sqlglotrs_crazy 13.0 ms 13.1 ms: ⚪ 1.01x slower
Geometric mean (ref) 1.00x slower

Benchmark hidden because not significant (6): parse_sqlglot_tpch, parse_sqlglot_short, parse_sqlglot_long, parse_sqlglot_crazy, parse_sqlglotrs_tpch, parse_sqlglotrs_long


Optimization Benchmark

Benchmark bench_optimize_main bench_optimize_pr
optimize_condition_100 125 ms 124 ms: ⚪ 1.01x faster
Geometric mean (ref) 1.00x faster

Benchmark hidden because not significant (3): optimize_tpch, optimize_condition_10, optimize_condition_1000

@github-actions
Copy link
Contributor

Benchmark Results

Legend:

  • 🟢🟢 = 2x+ faster
  • 🟢 = 1.1x - 2x faster
  • ⚪ = No significant change (< 1.1x)
  • 🔴 = 1.1x - 2x slower
  • 🔴🔴 = 2x+ slower

Parsing Benchmark

Benchmark bench_parse_main bench_parse_pr
parse_sqlglot_short 367 us 364 us: ⚪ 1.01x faster
parse_sqlglot_long 5.66 ms 5.61 ms: ⚪ 1.01x faster
parse_sqlglot_crazy 17.3 ms 17.6 ms: ⚪ 1.02x slower
parse_sqlglotrs_long 4.09 ms 4.13 ms: ⚪ 1.01x slower
Geometric mean (ref) 1.00x slower

Benchmark hidden because not significant (4): parse_sqlglot_tpch, parse_sqlglotrs_tpch, parse_sqlglotrs_short, parse_sqlglotrs_crazy


Optimization Benchmark

Benchmark bench_optimize_main bench_optimize_pr
optimize_tpch 900 ms 914 ms: ⚪ 1.02x slower
Geometric mean (ref) 1.01x slower

Benchmark hidden because not significant (3): optimize_condition_10, optimize_condition_100, optimize_condition_1000

@tobymao
Copy link
Owner Author

tobymao commented Nov 15, 2025

/benchmark

@github-actions
Copy link
Contributor

Benchmark Results

Legend:

  • 🟢🟢 = 2x+ faster
  • 🟢 = 1.1x - 2x faster
  • ⚪ = No significant change (< 1.1x)
  • 🔴 = 1.1x - 2x slower
  • 🔴🔴 = 2x+ slower

Parsing Benchmark

Benchmark hidden because not significant (8): parse_sqlglot_tpch, parse_sqlglot_short, parse_sqlglot_long, parse_sqlglot_crazy, parse_sqlglotrs_tpch, parse_sqlglotrs_short, parse_sqlglotrs_long, parse_sqlglotrs_crazy


Optimization Benchmark

Benchmark hidden because not significant (4): optimize_tpch, optimize_condition_10, optimize_condition_100, optimize_condition_1000


key = "expression"
arg_types = {"this": True}
required_args = {"this"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Owner Author

Choose a reason for hiding this comment

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

yea for type checks

@tobymao tobymao merged commit adcf14f into main Nov 15, 2025
7 checks passed
@tobymao tobymao deleted the toby/arg_cleanup branch November 15, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants