Skip to content

fix: refuse an input whose table would fall in the reserved _filesql_ namespace - #306

Merged
nao1215 merged 2 commits into
mainfrom
fix/reserve-filesql-table-prefix
Aug 9, 2026
Merged

fix: refuse an input whose table would fall in the reserved _filesql_ namespace#306
nao1215 merged 2 commits into
mainfrom
fix/reserve-filesql-table-prefix

Conversation

@nao1215

@nao1215 nao1215 commented Aug 9, 2026

Copy link
Copy Markdown
Owner

v0.43.0 began hiding _filesql_ tables from DumpDatabase and from the table listings this package returns, so the metadata table it writes does not show up as a user table. It did not stop a caller's input from landing in that namespace: a file named _filesql_report.csv still loaded, and the resulting table answered queries while being absent from every listing and from any dump — its rows were silently left out of an export, which is worse than the load failing.

A prefix is only reserved if nothing else can occupy it, so an input whose table name would begin with _filesql_ is now refused with ErrReservedTableName, naming the table and the prefix. SQLite answers the same way for its own sqlite_ prefix, and the rule follows that precedent. It covers every way a name is chosen: a path, a reader's explicit table name, an Excel sheet, and an ACH or Fedwire base name. A name that merely resembles the prefix, such as filesql_report, is unaffected — the check is a prefix match, not the LIKE pattern used for hiding.

This also makes the rule sqly relies on true: it can filter the prefix out of its own .tables listing knowing that no user table can be hidden by the filter.

Summary by CodeRabbit

  • New Features

    • Added validation for reserved table names beginning with _filesql_.
    • Added ErrReservedTableName to identify rejected table-name inputs.
    • Similar names without the leading underscore, such as filesql_report, remain valid.
  • Documentation

    • Updated ACH/Fedwire guidance and the changelog to explain reserved table-name rules.
  • Bug Fixes

    • Reserved names are rejected before data parsing, buffering, or database changes occur.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0c2856b6-7070-414b-84e9-33d4555632bc

📥 Commits

Reviewing files that changed from the base of the PR and between 2ecf0c4 and 5bc4dce.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • source_registry.go
  • source_registry_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • source_registry_test.go
  • CHANGELOG.md
  • source_registry.go

📝 Walkthrough

Walkthrough

The package now reserves table names beginning with _filesql_. It exposes ErrReservedTableName, validates names before ingestion or table creation, documents the rule, and tests rejection of reserved names alongside acceptance of lookalike names.

Changes

Reserved table names

Layer / File(s) Summary
Reserved-name contract and coverage
errors.go, source_registry.go, source_registry_test.go, CHANGELOG.md, README.md, ach.go
The package defines ErrReservedTableName, rejects names beginning with _filesql_, documents the reserved namespace, and tests reserved and lookalike names.
Ingestion-path validation
ach.go, wire.go, stream_processor.go
ACH, Fedwire, reader, and XLSX ingestion paths validate sanitized table names before parsing, buffering, database checks, or table creation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: rejecting inputs whose table names use the reserved _filesql_ namespace.
Description check ✅ Passed The description clearly explains the problem, intended behavior, affected input paths, error behavior, and valid lookalike names.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/reserve-filesql-table-prefix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

This comment has been minimized.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@source_registry.go`:
- Around line 176-180: Update validateTableName to normalize tableName and
compare it case-insensitively with sourceTablePrefix, matching SQLite LIKE
behavior while preserving the existing reserved-name error. Add a case-variant
regression case to TestReservedTableNameIsRefused.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cccd43fc-94f3-43f0-8551-16f56d4eb2c0

📥 Commits

Reviewing files that changed from the base of the PR and between 27bc6fe and 2ecf0c4.

📒 Files selected for processing (8)
  • CHANGELOG.md
  • README.md
  • ach.go
  • errors.go
  • source_registry.go
  • source_registry_test.go
  • stream_processor.go
  • wire.go

Comment thread source_registry.go
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Code Metrics Report

main (27bc6fe) #306 (d100907) +/-
Coverage 90.3% 90.3% -0.1%
Test Execution Time 14s 14s 0s
Details
  |                     | main (27bc6fe) | #306 (d100907) |  +/-  |
  |---------------------|----------------|----------------|-------|
- | Coverage            |          90.3% |          90.3% | -0.1% |
  |   Files             |             64 |             64 |     0 |
  |   Lines             |          10520 |          10535 |   +15 |
+ |   Covered           |           9507 |           9519 |   +12 |
  | Test Execution Time |            14s |            14s |    0s |

Code coverage of files in pull request scope (79.0% → 79.0%)

Files Coverage +/- Status
ach.go 86.8% -0.5% modified
errors.go 100.0% 0.0% modified
source_registry.go 87.5% +1.5% modified
stream_processor.go 73.4% +0.0% modified
wire.go 79.6% -1.1% modified

Reported by octocov

@nao1215
nao1215 merged commit 29952ef into main Aug 9, 2026
11 checks passed
@nao1215
nao1215 deleted the fix/reserve-filesql-table-prefix branch August 9, 2026 07:15
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.

1 participant