Skip to content

Fix: Prevent column names containing 'from' from breaking function names in views#220

Merged
PNixx merged 1 commit intoPNixx:masterfrom
DeweyLabs:prefixing-sql-functions
Nov 17, 2025
Merged

Fix: Prevent column names containing 'from' from breaking function names in views#220
PNixx merged 1 commit intoPNixx:masterfrom
DeweyLabs:prefixing-sql-functions

Conversation

@ScotterC
Copy link
Copy Markdown
Contributor

Problem

When creating views with create_table, the regex in assign_database_to_subquery! incorrectly matched 'from' in column names (like sourced_from), causing the next identifier to be prefixed with the database name.

Example:
SELECT sourced_from, now() FROM table

Would become:
SELECT sourced_from, default.now() FROM default.table

Error:
Function with name 'default.now' does not exist

Root Cause
The regex /(?<=from)[^.\w]+.../i used a lookbehind that matched 'from' anywhere in the SQL, not just the FROM keyword.

Solution
Changed the regex to /\bfrom\s+.../i using word boundary \b to match only the FROM keyword, not column names.

Testing

  • Added 2 tests specifically reproducing and verifying the fix
  • All 93 existing tests pass
  • Tested with ClickHouse 24.9 and 25.10

The regex in assign_database_to_subquery! used a lookbehind (?<=from)
which matched 'from' anywhere in the SQL, including in column names
like 'sourced_from'. This caused the next identifier to be incorrectly
prefixed with the database name.

Bug example:
  SELECT sourced_from, now() FROM table

Would become:
  SELECT sourced_from, default.now() FROM default.table

Causing error:
  Function with name 'default.now' does not exist

Fixed by using \bfrom\s+ to match 'from' only as a complete word
(SQL keyword), not as part of column names.

Added tests covering:
- Column names containing 'from'
- Verification that functions are not prefixed
- Verification that table names are correctly prefixed

All 93 tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@PNixx PNixx merged commit 3301f6b into PNixx:master Nov 17, 2025
20 checks passed
woodhull added a commit to daisychainapp/clickhouse-activerecord that referenced this pull request Nov 24, 2025
This merge brings in the latest improvements from upstream including:
- JSON column type support (PNixx#209)
- Fixed regex to match FROM keyword (PNixx#220)
- Active Record 8.1 support
- SQL structure dumper improvements
- Better format logic encapsulation (PNixx#162)
- Schema dumper fixes (PNixx#206)
- Unscope :final and :settings (PNixx#208)

Fork-specific functionality preserved:
- EOFError and Errno::ECONNRESET retry logic (integrated into raw_execute)
- Custom insert settings via ClickhouseActiverecord::Settings thread-local
- disconnect! method implementation
- Security fix: no hardcoded OpenSSL::SSL::VERIFY_NONE
- Async insert configuration support

The retry logic has been adapted to work with the new execute/raw_execute
architecture introduced by upstream, ensuring compatibility with the refactored
code structure while maintaining reliability for connection errors.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
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