Skip to content

Improved col_is{nt}_X implementation - #369

Open
RampantDespair wants to merge 1 commit into
theory:mainfrom
RampantDespair:dev1
Open

Improved col_is{nt}_X implementation#369
RampantDespair wants to merge 1 commit into
theory:mainfrom
RampantDespair:dev1

Conversation

@RampantDespair

@RampantDespair RampantDespair commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Closes #363 [1/3]

As discussed this is the first part of the PR split -> implementation symmetry.

All 3 now follow:

-- col_is{nt}_X( schema, table, column[], description ) -- ( NAME, NAME, NAME[], TEXT )
-- col_is{nt}_X( schema, table, column[] )              -- ( NAME, NAME, NAME[] )
-- col_is{nt}_X( table, column[], description )         -- ( NAME, NAME[], TEXT )
-- col_is{nt}_X( table, column[] )                      -- ( NAME, NAME[] )
-- col_is{nt}_X( schema, table, column, description )   -- ( NAME, NAME, NAME, TEXT )
-- col_is{nt}_X( schema, table, column )                -- ( NAME, NAME, NAME )
-- col_is{nt}_X( table, column, description )           -- ( NAME, NAME, TEXT )
-- col_is{nt}_X( table, column )                        -- ( NAME, NAME )

EDIT: col_is_null isn't in scope because you went the is/not instead of is/isnt so I didn't think it was pertinent. If you disagree I can conform it as well I suppose.

@theory

theory commented Jul 26, 2026

Copy link
Copy Markdown
Owner

EDIT: col_is_null isn't in scope because you went the is/not instead of is/isnt so I didn't think it was pertinent. If you disagree I can conform it as well I suppose.

No, I think it's the same, it's just named differently because SQL has a NOT NULL expression.

@RampantDespair

Copy link
Copy Markdown
Contributor Author

@theory Understood, I'll conform it right now, one moment

Comment thread doc/pgtap.md
Comment thread sql/pgtap.sql.in Outdated
Comment thread sql/pgtap.sql.in
Comment thread test/sql/fktap.sql
);

SELECT * FROM check_test(
col_is_fk( 'public', 'fk', 'pk_id'::name ),

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Oh, so the existing tests, which don't cast the third argument, still pass? Okay then it seems like it will be backward compatible after all.

Comment thread doc/pgtap.md
@theory

theory commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Also: Please use the 50/72 rule in the commit message.

@theory

theory commented Jul 26, 2026

Copy link
Copy Markdown
Owner

@theory Understood, I'll conform it right now, one moment

Sorry, don't know what this responds to.

@theory

theory commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Please rebase onto main and squash down to a single commit. Thanks.

@RampantDespair
RampantDespair force-pushed the dev1 branch 3 times, most recently from f2ac65c to 1f71852 Compare August 2, 2026 21:23
@RampantDespair

Copy link
Copy Markdown
Contributor Author

@theory
I think it's done.

Add schema-qualified overloads for column primary key and foreign key
assertions, and establish a consistent function order. Add
`col_isnt_unique()` with matching documentation and tests.

Standardize array-argument comments to use `columns[]`.
@RampantDespair

Copy link
Copy Markdown
Contributor Author

@theory
I believe this PR is complete, unless you have any further comments or requested changes.

Comment thread sql/pgtap.sql.in
Comment on lines -2065 to -2081
-- col_is_pk( schema, table, column[] )
CREATE OR REPLACE FUNCTION col_is_pk ( NAME, NAME, NAME[] )
RETURNS TEXT AS $$
SELECT col_is_pk( $1, $2, $3, 'Columns ' || quote_ident($1) || '.' || quote_ident($2) || '(' || _ident_array_to_string($3, ', ') || ') should be a primary key' );
$$ LANGUAGE sql;

-- col_is_pk( table, column[], description )
CREATE OR REPLACE FUNCTION col_is_pk ( NAME, NAME[], TEXT )
RETURNS TEXT AS $$
SELECT is( _ckeys( $1, 'p' ), $2, $3 );
$$ LANGUAGE sql;

-- col_is_pk( table, column[] )
CREATE OR REPLACE FUNCTION col_is_pk ( NAME, NAME[] )
RETURNS TEXT AS $$
SELECT col_is_pk( $1, $2, 'Columns ' || quote_ident($1) || '(' || _ident_array_to_string($2, ', ') || ') should be a primary key' );
$$ LANGUAGE sql;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please do not move functions. It creates unnecessary churn in the PR and complicates maintenance of the compatibility patches.

Comment thread sql/pgtap.sql.in
Comment on lines +2083 to +2088
-- col_is_pk( table, columns[], description )
CREATE OR REPLACE FUNCTION col_is_pk ( NAME, NAME[], TEXT )
RETURNS TEXT AS $$
SELECT is( _ckeys( $1, 'p' ), $2, $3 );
$$ LANGUAGE sql;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please put all the new functions together in contiguous lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants