Skip to content

Fix column quoting in incremental merge strategy for SQL keywords - #308

Open
mlilius wants to merge 5 commits into
dremio:mainfrom
mlilius:incremental-keyword-quotation
Open

Fix column quoting in incremental merge strategy for SQL keywords#308
mlilius wants to merge 5 commits into
dremio:mainfrom
mlilius:incremental-keyword-quotation

Conversation

@mlilius

@mlilius mlilius commented Aug 28, 2025

Copy link
Copy Markdown

Summary

Fix column quoting in incremental merge strategy for SQL keywords to prevent SQL errors when column names are SQL keywords.

Description

This PR addresses an issue where SQL keyword column names (like 'language', 'count', 'order', etc.) were not being properly quoted in incremental materialization merge strategies. When users had columns with these names, the generated SQL would fail because the column names weren't quoted.

Changes made:

  • Modified the incremental merge strategy SQL template to use adapter.quote(column_name) instead of raw column names
  • Added comprehensive test coverage for this functionality

Files changed:

  • dbt/include/dremio/macros/materializations/incremental/strategies.sql: Added proper column quoting
  • tests/functional/adapter/materialization/test_incremental.py: Added TestIncrementalColumnQuoting test class
  • CHANGELOG.md: Added entry for this fix

Test Results

Added new test TestIncrementalColumnQuoting.test_incremental_keyword_columns that validates:

  • Initial incremental run with SQL keyword columns ('language' and 'count') works correctly
  • Subsequent incremental updates with keyword columns work correctly
  • Proper row counts and data integrity after incremental updates
  • Correct handling of quoted column names in both initial and incremental runs

The test creates a model with columns named 'language' and 'count' (both SQL keywords) and verifies that incremental materialization works correctly with the merge strategy.

Changelog

  • Added a summary of what this PR accomplishes to CHANGELOG.md

Contributor License Agreement

@CLAassistant

CLAassistant commented Aug 28, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

- Quote column names in incremental merge strategy to handle SQL keywords
- Add comprehensive tests for incremental materialization with keyword columns
- Test validates proper handling of columns named 'language' and 'count'
- Ensures incremental updates work correctly with quoted column names
@mlilius
mlilius force-pushed the incremental-keyword-quotation branch from 0788e07 to 938dee2 Compare September 3, 2025 01:22
@ianils

ianils commented Oct 28, 2025

Copy link
Copy Markdown

@mlilius - looks like there are some conflicts ?

@mlilius

mlilius commented Nov 4, 2025

Copy link
Copy Markdown
Author

@ianils - the conflict is taken care of.

…ategy

- Updated the incremental merge strategy to quote unique keys and column names using the adapter's quote function, ensuring compatibility with SQL keywords.
- Adjusted the macro signature to include an optional parameter for incremental predicates, enhancing flexibility in SQL generation.
…essary quoting for column names. This change ensures that the column names are directly referenced, improving clarity and consistency in the SQL generation process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants