On Fri, Aug 15, 2025 at 12:36:00PM +0500, Kirill Reshke wrote: > 0001 - LGTM?
Thanks for the review. Yes, I am eyeing at applying this refactoring piece to limit the footprint of Form_pg_sequence_data. The expectations of reset_state are not completely documented at the top of init_params(), because it is not only about nextval(). I'll try to think about a more careful wording. > 0002. WFM, the sole observation I have is that we incorporate sequence > logic to AlterTable utilities, making it less clear of what they > actually do. Thanks, I am also looking at applying this part of the patch set. There is a second reason for that: we don't track yet what kind of attributes are created within a CREATE SEQUENCE, so ADD COLUMN TO SEQUENCE is IMO also useful on its own to make user-facing some of the actions taken internally by the backend in this case. Perhaps there's an argument that people do not like the AddColumnToView layer, and that we should not expand that to sequences, but I like this new separation concept for the deparsing logistics. > I see we already have code path handling CREATE VIEW (in > AlterTableGetLockLevel for example), so maybe it is worth renaming > AlterTable* to AlterRelation*? > It would be really invasive refactoring though, so maybe we should > ignore this slight inconsistency in function names and what they > actually do. Some of these AlterTable* sub-structures are also shared for some code paths of view, matviews and sequences, if I recall correctly. I'm used to this code using AlterTable things, so perhaps my opinion is just biased based on that. -- Michael
signature.asc
Description: PGP signature
