Skip to content

Conversation

Huliiiiii
Copy link
Member

Close #842

@Huliiiiii Huliiiiii requested review from tyt2y3 and Expurple September 1, 2025 01:05
Comment on lines 138 to 144
/// NEW.*
New(DynIden),
/// Old.*
Old(DynIden),
#[cfg(feature = "backend-postgres")]
/// excluded.*
Excluded(DynIden),
Copy link
Member

Choose a reason for hiding this comment

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

To we really need these special variants? These table names look like they can be already expressed as "regular" table names: ColumnRef::from(("OLD", MyColumn))

It may be enough to define some unit-struct idens for these tables and document the approach.

Copy link
Member Author

Choose a reason for hiding this comment

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

prepare_iden will quote them, but they cannot be quoted.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I see. Then, I wouldn't change ColumnRef at all. We don't need to do anything with the "column basename" part. We need to somehow allow unquoted TableNames (which would then be stored in ColumnRef as usual) and provide a way to construct unquoted TableNames ergonomically

Copy link
Member

@Expurple Expurple left a comment

Choose a reason for hiding this comment

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

Also, your current approach doesn't support asterisk, like NEW.* (literally * in the query)

@Huliiiiii
Copy link
Member Author

Also, your current approach doesn't support asterisk, like NEW.* (literally * in the query)

This means the glob pattern, not the SQL asterisk.

@Expurple
Copy link
Member

Expurple commented Sep 2, 2025

I understand. I'm pointing out that the SQL asterisk is also valid with these tables. I'd like to find a clean way to support it right away

@Huliiiiii
Copy link
Member Author

I think we could add an enum of pseudo tables/columns and regular idens.
Basically it’s just raw idens vs regular ones.
One hacky approach is to check in prepare_iden if the iden is one of the specific keywords.

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.

No way to express "excluded" in upsert where clause in SeaQL
2 participants