Skip to content

Fix prepared-statement name collisions, row members named like row hooks, and CamelCase query file classes - #55

Merged
devin-ai-integration[bot] merged 2 commits into
mainfrom
devin/1793754636-sweep-naming-fixes
Nov 4, 2026
Merged

devin-ai-integration[bot] merged 2 commits into
mainfrom
devin/1793754636-sweep-naming-fixes

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes three generator bugs found in a pre-release sweep. Each one has a test that fails on main. The PR also fixes or documents the suspicions from that sweep.

1. PostgreSQL prepared-statement names could collide (High). Names were snake_cased and joined with _, so different class paths gave the same name. Options::PgV2::FirstSql and Options::Pg::V2::FirstSql both got options_pg_v2_first_sql_…, and names that PostgreSQL truncated at 63 bytes could also match. Two classes sharing a pool then ran each other's SQL or raised PG::DuplicatePstatement.

  • Fix: Code.statement_name names each statement Module::Class#method. That name is unambiguous because :: and # never appear inside a constant or method name. Names over 63 bytes become a prefix, ~, and a 64-bit FNV-1a hash of the full name.
  • Tests: tests/end2end_options.rb adds module pairs Options::PgV2 / Options::Pg::V2 and two long module names that share their first 63 bytes. Each pair runs on one ConnectionPool. Unit test: test_statement_names_of_different_classes_never_collide.

2. Columns named like private row hooks broke every row (Low-Med). A column named initialize (also initialize_copy, initialize_clone, initialize_dup, method_missing) was not renamed, so Data / Struct construction raised ArgumentError.

  • Fix: these names are added to the reserved row-method names (Code::CONSTRUCTION_METHODS), so they get a trailing _ (initialize_).
  • Tests: the example schemas of all three engines get an initialize column in name_cases. test_keyword_and_method_names_keep_distinct_row_and_argument_names reads it back as initialize_. Unit test test_columns_named_like_row_methods_still_build_and_read_rows goes through every public and private Struct / Data method on the running Ruby, then builds, reads, dups, clones and withs a row for each one.

3. Query file names were not CamelCased (Low). admin_users.sql generated Admin_usersSql, which is a RuboCop Naming/ClassAndModuleCamelCase offense.

  • Fix: Code.constant_part CamelCases file names, so admin_users.sql becomes AdminUsersSql in admin_users_sql.rb. Two files that map to the same class are still rejected, and the error now names the files.
  • Tests: the options E2E adds sqlite_queries/admin_users.sql and runs AdminUsersSql. Unit tests: test_query_file_names_become_camel_case_classes and the updated test_query_files_writing_the_same_file_are_rejected.

Breaking change: a query file with _ in its name now generates a different class name. For example, my_queries.sql used to give My_queriesSql and now gives MyQueriesSql. Callers have to rename the class. File names stay the same.

Suspicions from the sweep

  • rubyVersion: "3" generated Data, which breaks on Ruby 3.1. A missing minor version now counts as .0 for this check, so 3 generates Structs (test_immutable_data_support).
  • MySQL :execrows counts changed rows, not matched rows (mysql2's default). This is now documented with the flags: Mysql2::Client::FOUND_ROWS alternative (test_execrows_counts_changed_rows_unless_found_rows_is_set).
  • A String passed to a PostgreSQL json / jsonb parameter is JSON text. This is now documented; the existing JSON round-trip test already covers it.
  • SQLite busy_timeout= waits while holding the GVL, so other threads stop. This is now documented, recommending busy_handler_timeout= (test_busy_handler_timeout_waits_for_another_connection_without_blocking_threads).
  • SQLite booleans: no change. The generated RBS already uses Integer, and passing true is a sqlite3 gem limitation.

examples/, test/golden/, test/fixtures/*.bin and steep_expectations.yml are regenerated (only line numbers moved in the Steep expectations). README, docs/Coverage.md and docs/Testing.md are updated.

Checklist

  • examples/ is regenerated (rake sqlc_generate_process) and committed
  • Tests added or updated (rake test, and rake end2end_* when the generated code changes)
  • The generated code stays RuboCop-clean (rake rubocop) and Steep-clean (rake steep)
  • Labelled (bug, enhancement, documentation, or breaking) so the release notes are grouped correctly

devin-ai-integration Bot and others added 2 commits November 4, 2026 01:16
…ision-free

Co-Authored-By: Doron Eli Rachman <doron050@gmail.com>
…QLite busy waiting

Co-Authored-By: Doron Eli Rachman <doron050@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot added the bug Something isn't working label Nov 4, 2026
@devin-ai-integration
devin-ai-integration Bot merged commit 6ea4f05 into main Nov 4, 2026
41 checks passed
@devin-ai-integration
devin-ai-integration Bot deleted the devin/1793754636-sweep-naming-fixes branch November 4, 2026 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant