Skip to content

Comments

Scd2 incremental key column#1712

Merged
karakanb merged 9 commits intomainfrom
cursor/scd2-incremental-key-column-d091
Feb 22, 2026
Merged

Scd2 incremental key column#1712
karakanb merged 9 commits intomainfrom
cursor/scd2-incremental-key-column-d091

Conversation

@sabrikaragonen
Copy link
Contributor

Enable using a specified column via incremental_key for _valid_from and _valid_until fields in scd2_by_column materialization.


Slack Thread

Open in Web Open in Cursor 

@cursor
Copy link

cursor bot commented Feb 20, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@karakanb karakanb marked this pull request as ready for review February 22, 2026 15:43
@karakanb karakanb merged commit 3ca63ef into main Feb 22, 2026
9 checks passed
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

28 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile


validFromExpr := "CURRENT_TIMESTAMP"
if asset.Materialization.IncrementalKey != "" {
validFromExpr = QuoteIdentifier(asset.Materialization.IncrementalKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

The quoted column name should be prefixed with the table alias for clarity

Suggested change
validFromExpr = QuoteIdentifier(asset.Materialization.IncrementalKey)
validFromExpr = "src." + QuoteIdentifier(asset.Materialization.IncrementalKey)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/postgres/materialization.go
Line: 332

Comment:
The quoted column name should be prefixed with the table alias for clarity

```suggestion
		validFromExpr = "src." + QuoteIdentifier(asset.Materialization.IncrementalKey)
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.


validFromExpr := "CURRENT_TIMESTAMP()"
if asset.Materialization.IncrementalKey != "" {
validFromExpr = asset.Materialization.IncrementalKey
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing table alias prefix and type casting

Suggested change
validFromExpr = asset.Materialization.IncrementalKey
validFromExpr = fmt.Sprintf("CAST(src.%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/databricks/materialization.go
Line: 238

Comment:
Missing table alias prefix and type casting

```suggestion
		validFromExpr = fmt.Sprintf("CAST(src.%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
```

How can I resolve this? If you propose a fix, please make it concise.


validFromExpr := "CURRENT_TIMESTAMP()"
if asset.Materialization.IncrementalKey != "" {
validFromExpr = fmt.Sprintf("CAST (%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing src. table alias prefix - column reference will fail

Suggested change
validFromExpr = fmt.Sprintf("CAST (%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
validFromExpr = fmt.Sprintf("CAST (src.%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/bigquery/materialization.go
Line: 523

Comment:
Missing `src.` table alias prefix - column reference will fail

```suggestion
		validFromExpr = fmt.Sprintf("CAST (src.%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
```

How can I resolve this? If you propose a fix, please make it concise.


validFromExpr := "CURRENT_TIMESTAMP()"
if asset.Materialization.IncrementalKey != "" {
validFromExpr = fmt.Sprintf("CAST(%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing src. table alias prefix - column reference will fail

Suggested change
validFromExpr = fmt.Sprintf("CAST(%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
validFromExpr = fmt.Sprintf("CAST(src.%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/snowflake/materialization.go
Line: 395

Comment:
Missing `src.` table alias prefix - column reference will fail

```suggestion
		validFromExpr = fmt.Sprintf("CAST(src.%s AS TIMESTAMP)", asset.Materialization.IncrementalKey)
```

How can I resolve this? If you propose a fix, please make it concise.

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.

3 participants