Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DBZ-5398 The column is referenced as PRIMARY KEY, but a matching column is not defined in table #3718

Merged
merged 1 commit into from
Jul 25, 2022

Conversation

harveyyue
Copy link
Contributor

https://issues.redhat.com/browse/DBZ-5398
@jpechane @Naros I couldn't find a graceful solution to parse the column name from index definition including a function. Currently, the regex expression just support the function including parameters which split by comma, and the nested function also not be supported as below.
coalesce(coalesce(col1,'1'),'xxx')

@Naros
Copy link
Member

Naros commented Jul 14, 2022

I can't help but wonder if this is over-engineered and could lead to other problems. This may not apply to PG but at least for other databases, there are situations where the PK consists of a function call that wraps a synthetic column that isn't naturally available in the JDBC metadata, i.e. ROWID or some other "hidden" column. I believe PG has similar things like CTID.

So I'm inclined to question whether a better solution for this would have been to flag that the PK/index tuple we identified should be excluded entirely as a unique index or primary key mapping like we do for Oracle.

@harveyyue
Copy link
Contributor Author

@Naros You means we can exclude the entire index columns if including function expression according method isTableUniqueIndexIncluded. I know the following index columns are not database internal or "hidden" column.
However, I will imeplement the solution as you sugguest.

Create Index Statement:
create unique index counter_ukey on counter(campaign_id, COALESCE(group_id, ''::text));

@Naros
Copy link
Member

Naros commented Jul 18, 2022

Yes @harveyyue, I most raised the question as a means to be consistent. I'll defer to @jpechane if he's okay with this pattern but I think in general I'd prefer consistency.

@jpechane
Copy link
Contributor

@harveyyue I am fine with this soution but will it capture siatuatilons like index being defined as for example col1 + col2 as I suppose there can be not only function call but arbitrary expression.

@jpechane
Copy link
Contributor

Could you please add a test cae similar to #3728 ?

@harveyyue
Copy link
Contributor Author

Could you please add a test cae similar to #3728 ?

Ok, will do it.

@harveyyue
Copy link
Contributor Author

harveyyue commented Jul 24, 2022

@jpechane Add a expression regex including these operators + - * / < > = ~ ! @ # % ^ & | ?`, also add a expression test case.
https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-OPERATORS

@Naros Naros self-requested a review July 25, 2022 12:43
Copy link
Member

@Naros Naros left a comment

Choose a reason for hiding this comment

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

Overall this LGTM @harveyyue, thanks a lot for this!

@Naros Naros merged commit 187ccb2 into debezium:main Jul 25, 2022
@Naros
Copy link
Member

Naros commented Jul 25, 2022

Applied to main (=2.0).

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