Issue tbd wrap pg proc test functions #2805
Conversation
WalkthroughThis update refactors multiple SQL test scripts in the pgRouting pgtap suite. All changes replace legacy OID‐based function argument type checks (using queries with Changes
Sequence Diagram(s)sequenceDiagram
participant TS as Test Script
participant FT as function_types_eq
participant PG as pg_proc Catalog
TS->>FT: Call function_types_eq(fn, expectedTypes)
FT->>PG: Query function signature (proallargtypes)
PG-->>FT: Return actual type array
FT-->>TS: Compare actual vs expected (TEXT arrays)
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (74)
pgtap/allpairs/floydWarshall/types_check.pg(1 hunks)pgtap/allpairs/johnson/types_check.pg(1 hunks)pgtap/alpha_shape/types_check.pg(1 hunks)pgtap/bdDijkstra/bdDijkstraCost/types_check.pg(2 hunks)pgtap/bdDijkstra/bdDijkstraCostMatrix/types_check.pg(1 hunks)pgtap/bellman_ford/bellman_ford/types_check.pg(2 hunks)pgtap/bellman_ford/edwardMoore/types_check.pg(2 hunks)pgtap/chinese/chinesePostman/types_check.pg(1 hunks)pgtap/chinese/chinesePostmanCost/types_check.pg(1 hunks)pgtap/circuits/hawickCircuits/types_check.pg(1 hunks)pgtap/coloring/bipartite/types_check.pg(1 hunks)pgtap/coloring/edgeColoring/types_check.pg(1 hunks)pgtap/coloring/sequentialVertexColoring/types_check.pg(1 hunks)pgtap/components/articulationPoints/types_check.pg(1 hunks)pgtap/components/biconnectedComponents/types_check.pg(1 hunks)pgtap/components/bridges/types_check.pg(1 hunks)pgtap/components/connectedComponenes/types_check.pg(1 hunks)pgtap/components/makeConnected/types_check.pg(1 hunks)pgtap/components/strongComponenets/types_check.pg(1 hunks)pgtap/contraction/contraction/types_check.pg(2 hunks)pgtap/contraction/contractionDeadEnd/types_check.pg(1 hunks)pgtap/contraction/contractionLinear/types_check.pg(1 hunks)pgtap/dijkstra/dijkstraCost/types_check.pg(2 hunks)pgtap/dijkstra/dijkstraCostMatrix/types_check.pg(1 hunks)pgtap/dijkstra/dijkstraNear/types_check.pg(1 hunks)pgtap/dijkstra/dijkstraNearCost/types_check.pg(1 hunks)pgtap/dijkstra/driving_distance/types_check.pg(2 hunks)pgtap/dominator/lengauerTarjanDominatorTree/types_check.pg(1 hunks)pgtap/ksp/ksp/types_check.pg(2 hunks)pgtap/ksp/turnRestrictedPath/types_check.pg(1 hunks)pgtap/ksp/withPointsKSP/types_check.pg(1 hunks)pgtap/lineGraph/lineGraph/types_check.pg(1 hunks)pgtap/lineGraph/lineGraphFull/types_check.pg(1 hunks)pgtap/max_flow/edgedisjointpaths/types_check.pg(2 hunks)pgtap/max_flow/maxCardinalityMatch/types_check.pg(2 hunks)pgtap/max_flow/maxFlow/types_check.pg(2 hunks)pgtap/max_flow/maxFlowMinCost/types_check.pg(2 hunks)pgtap/max_flow/maxFlowMinCost_Cost/types_check.pg(2 hunks)pgtap/metrics/betweennessCentrality/types_check.pg(1 hunks)pgtap/metrics/degree/types_check.pg(1 hunks)pgtap/mincut/stoerWagner/types_check.pg(1 hunks)pgtap/ordering/cuthillMckeeOrdering/types_check.pg(1 hunks)pgtap/ordering/topologicalSort/inner_query.pg(1 hunks)pgtap/ordering/topologicalSort/no_crash_test.pg(1 hunks)pgtap/ordering/topologicalSort/types_check.pg(1 hunks)pgtap/pickDeliver/pickDeliver/types_check.pg(1 hunks)pgtap/pickDeliver/pickDeliverEuclidean/types_check.pg(1 hunks)pgtap/topology/analyzeGraph/types_check.pg(1 hunks)pgtap/topology/analyzeOneWay/types_check.pg(1 hunks)pgtap/topology/createTopology/types_check.pg(1 hunks)pgtap/topology/createVerticesTable/types_check.pg(1 hunks)pgtap/topology/extractVertices/types_check.pg(1 hunks)pgtap/topology/nodeNetwork/types_check.pg(1 hunks)pgtap/transitiveClosure/types_check.pg(1 hunks)pgtap/traversal/binaryBreadthFirstSearch/types_check.pg(2 hunks)pgtap/traversal/breadthFirstSearch/types_check.pg(1 hunks)pgtap/traversal/depthFirstSearch/types_check.pg(1 hunks)pgtap/trsp/trsp/types_check.pg(1 hunks)pgtap/trsp/trspViaEdges/types_check.pg(1 hunks)pgtap/trsp/trspViaVertices/types_check.pg(1 hunks)pgtap/utilities/findCloseEdges/types_check.pg(2 hunks)pgtap/version/full_version/types_check.pg(1 hunks)pgtap/version/version/types_check.pg(1 hunks)pgtap/vrp_basic/types_check.pg(1 hunks)pgtap/withPoints/withPoints/types_check.pg(2 hunks)pgtap/withPoints/withPointsCost/types_check.pg(2 hunks)pgtap/withPoints/withPointsCostMatrix/types_check.pg(1 hunks)pgtap/withPoints/withPointsDD/types_check.pg(2 hunks)tools/testers/astar_pgtap_tests.sql(9 hunks)tools/testers/dijkstra_pgtap_tests.sql(3 hunks)tools/testers/flow_pgtap_tests.sql(2 hunks)tools/testers/general_pgtap_tests.sql(1 hunks)tools/testers/spanningtree.sql(5 hunks)tools/testers/tsp_pgtap_tests.sql(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Ubuntu Boost (86)
- GitHub Check: Ubuntu Boost (79)
- GitHub Check: Ubuntu Boost (56)
- GitHub Check: Ubuntu Boost (84)
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: Ubuntu Boost (83)
- GitHub Check: Ubuntu Boost (77)
- GitHub Check: Ubuntu Boost (76)
- GitHub Check: Ubuntu Boost (80)
- GitHub Check: Ubuntu Boost (78)
- GitHub Check: Ubuntu Boost (68)
- GitHub Check: macos (macos-14)
- GitHub Check: macos (macos-latest)
- GitHub Check: Ubuntu clang
🔇 Additional comments (104)
pgtap/allpairs/floydWarshall/types_check.pg (1)
34-37: Improved type checking with better readabilityThe change replaces OID-based argument type checking with the
function_types_eqfunction and human-readable type names. This improves readability and maintainability by using explicit type names (text,bool,int8,int8,float8) instead of numeric OIDs.pgtap/dijkstra/dijkstraCostMatrix/types_check.pg (1)
34-37: Improved type checking mechanismThe change replaces the previous OID-based type checking with the more readable
function_types_eqfunction. Using explicit type names (text,anyarray,bool,int8,int8,float8) instead of numeric OIDs makes the test more maintainable and easier to understand.pgtap/ordering/topologicalSort/inner_query.pg (1)
1-29: Well-structured new test file for topological sortThis new test file follows the project's conventions with proper licensing, transaction management, and test structure. The file sets up the test environment by modifying edge costs and executing the topological sort function through the
style_dijkstrahelper.pgtap/chinese/chinesePostmanCost/types_check.pg (1)
39-41: Enhanced type checking with better readabilityThe change replaces the previous OID-based type checking with the more explicit
function_types_eqfunction. Using a text array ({text}) for type specification improves readability and maintainability compared to numeric OIDs.pgtap/version/full_version/types_check.pg (1)
37-38: Well-structured type check refactoringThe change from using
set_eqwith OID-based parameter types to usingfunction_types_eqwith human-readable text types is a good improvement. This makes the test more readable and maintainable since it's now immediately clear thatpgr_full_versionexpects 9 text parameters without having to interpret numeric OID values.pgtap/allpairs/johnson/types_check.pg (1)
34-36: Improved type checking clarityNice replacement of OID-based type checking with human-readable type names. The new approach using
function_types_eqwith explicit type names (text,bool,int8,int8,float8) is much more readable and maintainable than using numeric OIDs.pgtap/version/version/types_check.pg (1)
24-24:❓ Verification inconclusive
Verify test reduction is intentional
The number of planned tests has been reduced from 6 to 3, which suggests that some tests have been removed entirely rather than refactored like in the other files. Could you clarify why the tests for argument names, argument types, and return type have been removed instead of being updated to use
function_types_eq?
🏁 Script executed:
#!/bin/bash # Check what tests were removed from this file by looking at git history git show HEAD~1:pgtap/version/version/types_check.pg | grep -A 10 "SELECT plan"Length of output: 281
Action: Please Confirm If Test Reduction in types_check.pg Is Intentional
It looks like the file now only plans for three tests, and based on the git history from HEAD~1, tests checking for function existence and the return type remain while any explicit tests for argument names and argument types are no longer present. Could you clarify whether this removal was deliberate—that is, if these checks are now being handled by a consolidated approach (e.g., via
function_types_eq)—or if additional tests are meant to be added?
- Previously (HEAD~1), the file ran checks for existence and return type.
- The planned test count now reflects only 3 tests, with no separate checks for argument names/types.
- Please confirm if this change aligns with the refactoring strategy in other files.
pgtap/components/articulationPoints/types_check.pg (1)
34-35: Good migration to readable type checksThis change effectively moves from OID-based type checking to using
function_types_eqwith human-readable type names (text,int8). This approach aligns with the changes in other files and improves code readability and maintainability.pgtap/pickDeliver/pickDeliverEuclidean/types_check.pg (1)
37-40: Improved type checking with human-readable formatThis change replaces the previous OID-based type check with a more readable text representation of argument types. The
function_types_eqfunction provides a clearer way to validate the argument types forpgr_pickdelivereuclidean, which should make maintenance easier.pgtap/lineGraph/lineGraphFull/types_check.pg (1)
33-34: Enhanced readability with text-based type checkingConverting from OID-based type checking (
set_eq) to the more explicitfunction_types_eqwith human-readable type names (text,int4,int8, etc.) improves code clarity and maintainability. This change aligns with the standardization across the codebase.pgtap/components/connectedComponenes/types_check.pg (1)
33-34: Standardized type checking approachGood refactoring from OID-based type checking to text-based checking with
function_types_eq. This makes the expected argument types forpgr_connectedcomponentsmore readable and follows the consistent pattern being applied across the test suite.pgtap/components/bridges/types_check.pg (1)
34-35: Consistent approach to type validationThe change to
function_types_eqwith readable type names (text,int8) improves code clarity over the previous OID-based comparison. This follows the same pattern as other files in this update and helps standardize the testing approach.pgtap/pickDeliver/pickDeliver/types_check.pg (1)
36-39: Improved type checking for pgr_pickdeliver functionThis change replaces the old OID-based type checking with the more readable
function_types_eqapproach. Using text representations of types (text, float8, int4, etc.) instead of numeric OIDs significantly improves readability and maintainability of the test code.pgtap/components/makeConnected/types_check.pg (1)
45-47: Better type validation for pgr_makeconnectedGood refactoring from OID-based type checking to human-readable type names using
function_types_eq. The change maintains the same functionality while making the test more maintainable and easier to understand. The array format '{text,int8,int8,int8}' clearly communicates the expected argument types.pgtap/lineGraph/lineGraph/types_check.pg (1)
33-34: Improved type checking clarity for pgr_linegraphThis change follows the same pattern as the other files, replacing OID-based type checking with the more readable and maintainable
function_types_eqapproach. The TEXT array of types is much easier to understand than numeric OIDs.pgtap/vrp_basic/types_check.pg (1)
32-33: Enhanced type validation for pgr_vrponedepotThis change completes the consistent pattern of refactoring across the test suite. The transition from OID-based type checking to using
function_types_eqwith readable type names improves code clarity and maintainability.pgtap/coloring/sequentialVertexColoring/types_check.pg (1)
46-48: Improved type checking readability with function_types_eqThe change from using
set_eqwith OID-based type checks tofunction_types_eqwith human-readable type names is a good improvement. Using{text,int8,int8}instead of OIDs makes the test more maintainable and easier to understand.pgtap/coloring/bipartite/types_check.pg (1)
46-48: Improved type checking readability with function_types_eqGood improvement replacing the OID-based type checks with human-readable type names (
{text,int8,int8}). This approach is more maintainable and consistent with the pattern being applied across the codebase.pgtap/topology/createVerticesTable/types_check.pg (1)
33-35: Enhanced type checking with function_types_eqThe change to use
function_types_eqwith human-readable type names ({text,text,text,text,text}) is a good improvement for maintainability. This brings consistency with the type checking pattern applied across other test files.pgtap/contraction/contractionDeadEnd/types_check.pg (1)
43-44: Function call standardized to function_types_eqGood change standardizing on
function_types_eqfor type checking. This creates consistency across the test suite while maintaining the already human-readable type specifications.pgtap/topology/analyzeOneWay/types_check.pg (1)
33-34: Improved type checking approachThe code now uses
function_types_eqinstead of direct OID comparisons for checking function parameter types. This change enhances readability by using human-readable type names (like 'text', 'bool') instead of numeric OIDs.pgtap/topology/createTopology/types_check.pg (1)
33-34: Enhanced readability with function_types_eqThe change replaces a direct OID comparison with the more readable
function_types_eqfunction. This makes the test more maintainable by using text-based type names ('text', 'float8', 'bool') rather than numeric OIDs.pgtap/max_flow/maxFlow/types_check.pg (3)
23-23: Updated test countThe plan count has been reduced from 14 to 13 tests. Make sure this aligns with the actual number of tests being executed in the script.
54-61: Improved type checking for version 3.2.0+The code now uses
function_types_eqwith human-readable type names instead of OID arrays. This change makes the tests more readable and maintainable, clearly showing the expected function signatures.
65-71: Enhanced type checking for older versionsSimilar to the 3.2.0+ version, this section now uses
function_types_eqwith readable type names for older PostgreSQL versions. Consistent approach across version-specific code paths.pgtap/withPoints/withPointsCostMatrix/types_check.pg (1)
34-36: Improved parameter type checkingThe test now uses
function_types_eqwith a TEXT array of human-readable type names instead of numeric OIDs. This makes the test more intuitive and easier to maintain.pgtap/mincut/stoerWagner/types_check.pg (1)
36-39: Good refactoring to improve readability and maintainabilityThe change from using a direct query against
pg_procto usingfunction_types_eqmakes the code more maintainable. The new approach with human-readable TEXT arrays (e.g., 'text', 'int4', etc.) instead of OID values is much easier to understand and maintain.pgtap/ksp/turnRestrictedPath/types_check.pg (1)
34-35: Improved function type checking approachThe change from using
set_eqwith a query to thepg_proccatalog to using thefunction_types_eqhelper function is a good improvement. The human-readable type specifications make the tests more maintainable and easier to understand than OID-based checks.pgtap/ordering/cuthillMckeeOrdering/types_check.pg (1)
46-49: Good standardization of type checkingThis refactoring consistently applies the new pattern of using
function_types_eqwith human-readable type arrays. The improved readability will make these tests easier to maintain long-term.pgtap/topology/nodeNetwork/types_check.pg (1)
33-35: Consistent implementation of improved type checkingThis change follows the same pattern as the other files, replacing OID-based type checking with human-readable type names. The standardization across the codebase improves maintainability.
pgtap/trsp/trspViaEdges/types_check.pg (1)
38-41: Improvement in type checking clarityThe change from using
set_eqwith a pg_proc query tofunction_types_eqwith explicit type names improves code readability and maintainability. Using human-readable TEXT array representation (text,_int4,_float8,bool,bool,text,int4,int4,int4,int4,float8) instead of numeric OIDs makes the expected function signature much clearer to understand.pgtap/transitiveClosure/types_check.pg (1)
40-43: Enhanced type checking with clear type namesThe refactoring replaces the OID-based approach with a more readable type checking method using
function_types_eq. The TEXT array representation makes it immediately clear what types the function expects (text,int4,int8,_int8), improving test maintainability.pgtap/ordering/topologicalSort/types_check.pg (1)
1-54: Well-structured test for topological sort functionalityThis new test file for
pgr_topologicalsortfollows the established pattern used in other type checking tests. It properly validates the function's existence, argument types, and return type using the improvedfunction_types_eqapproach with explicit type names rather than OIDs.pgtap/coloring/edgeColoring/types_check.pg (1)
46-49: Consistent improvement in type checking readabilityThis change continues the pattern of replacing OID-based checks with the more readable
function_types_eqapproach. The TEXT array representation (text,int8,int8) makes the expected argument types immediately clear, improving test maintainability.pgtap/alpha_shape/types_check.pg (1)
27-29: Improved type checking approach.The change from using a CTE with
set_eqto usingfunction_types_eqdirectly is a good improvement. This simplifies the code while maintaining the same functionality. The use of human-readable TEXT arrays (geometry,float8) instead of OIDs makes the tests more maintainable.Note that the comment on line 26 saying "pgtap does not like geometry type, this is a workaround" might be obsolete now if
function_types_eqhandles geometry types properly.pgtap/contraction/contractionLinear/types_check.pg (1)
43-44: Standardized type checking approach.The change to use
function_types_eqis consistent with the standardization of type checking across the codebase. This makes the code more maintainable by using a dedicated function for type checking rather than raw SQL queries withset_eq.pgtap/components/biconnectedComponents/types_check.pg (1)
34-36: Enhanced readability with text-based type checking.Replacing OID-based type checking with human-readable text types (
text,int8,int8,int8) improves code clarity and maintainability. Thefunction_types_eqapproach is more direct and easier to understand than the previousset_eqwith pg_proc queries.pgtap/topology/extractVertices/types_check.pg (1)
33-34: Good refactoring to use human-readable type names.The change from using a
set_eqwith OID values tofunction_types_eqwith human-readable type names (text,bool,int8, etc.) improves code readability and maintainability. This approach makes it much clearer what type signatures are being validated.pgtap/ksp/ksp/types_check.pg (2)
54-60: Good upgrade to use human-readable type names instead of OIDs.This refactoring replaces OID-based type checking with explicit human-readable type names. The change makes the test more maintainable and easier to understand by directly specifying PostgreSQL data types like
text,int8,bool, etc. instead of numeric OIDs.
70-71: Improved readability with text-based type names.Good change to use explicit text representation of PostgreSQL types instead of OIDs in the else branch as well. Consistent with the approach used in the conditional branch above.
pgtap/bdDijkstra/bdDijkstraCostMatrix/types_check.pg (1)
34-36: Improved type checking with descriptive type names.The change from using OID-based type checking to explicit human-readable type names makes the test more maintainable. The new approach with
function_types_equsing clear type names liketext,anyarray,boolis much easier to understand and maintain.pgtap/circuits/hawickCircuits/types_check.pg (1)
50-52: Better type validation with human-readable formats.The switch from OID-based type validation to descriptive TEXT array with
function_types_eqimproves code readability and maintainability. The explicit type names make it much clearer what the expected function signature should be.pgtap/chinese/chinesePostman/types_check.pg (1)
34-34: Improved type checking with function_types_eqThe change from using a raw query with
set_eqto a more specializedfunction_types_eqfunction improves readability and maintainability. The human-readable TEXT array representation of types (text, int4, int8, etc.) is much clearer than using numeric OIDs, making the tests easier to understand and maintain.pgtap/dominator/lengauerTarjanDominatorTree/types_check.pg (1)
46-49: Improved type checking implementationThe modification correctly changes the type checking from a custom query to a dedicated
function_types_eqfunction, using human-readable type names like 'text' and 'int8' rather than numeric OIDs. This makes the test more maintainable and consistent with the project's modernization efforts.pgtap/trsp/trspViaVertices/types_check.pg (1)
37-40: Improved type checking with standardized approachThe modification correctly replaces the database query approach with the dedicated
function_types_eqfunction, using human-readable type names. This enhances readability and maintainability of the test by making the expected function signature more explicit.pgtap/traversal/depthFirstSearch/types_check.pg (1)
51-56: Improved type checking with better readability for multiple signaturesThe modification successfully replaces OID-based type checking with the clearer
function_types_eqapproach using human-readable type names. The code effectively handles checking both function signatures (single vertex and array of vertices inputs) in a more maintainable way.pgtap/metrics/betweennessCentrality/types_check.pg (1)
50-52: Improved type checking with human-readable typesThe switch from using a direct query against
pg_procto using thefunction_types_eqfunction with text-based type definitions improves code readability and maintainability. Using human-readable type names (text,bool,int8,float8) instead of OIDs makes the expected function signature much clearer.pgtap/withPoints/withPoints/types_check.pg (2)
63-70: Enhanced type validation with descriptive type namesThe refactoring to use
function_types_eqwith human-readable type names is a significant improvement. This change makes the test more maintainable by:
- Using clear type names (
text,int8,bool, etc.) instead of cryptic OID numbers- Maintaining consistency with other type checks in the codebase
- Simplifying the validation logic
The conditional block structure is properly maintained to handle version differences.
83-89: Consistent approach for older versionsGood implementation of the same pattern for older versions, ensuring consistent type checking regardless of pgRouting version. This maintains backward compatibility while improving readability.
pgtap/trsp/trsp/types_check.pg (1)
40-49: Standardized function argument type checkingGreat refactoring from
bag_hastofunction_types_eq, which aligns with the project-wide approach to type validation. The human-readable type format makes the test more maintainable and the expected function signatures easier to understand.Multiple function signatures are properly defined to cover all variants of the
pgr_trspfunction, ensuring comprehensive validation.pgtap/traversal/breadthFirstSearch/types_check.pg (1)
41-46: Enhanced type checking with descriptive type namesThe switch to
function_types_eqwith human-readable type names improves test clarity and maintainability. This change eliminates the need to look up OID values and makes the expected function signatures immediately clear to developers.Both single-value and array-based parameter variants are properly defined in the test.
pgtap/max_flow/maxFlowMinCost/types_check.pg (1)
63-71: Improved type checking with better readabilityThe change from direct SQL queries with
set_eqto usingfunction_types_eqwith human-readable TEXT arrays improves code maintainability. Type names like 'text' and 'int8' are more intuitive than OID values, making the tests easier to understand and maintain.pgtap/dijkstra/dijkstraCost/types_check.pg (1)
63-70: Streamlined type checking with descriptive type namesGood replacement of OID-based checks with the more direct
function_types_eqapproach. This change improves readability by using descriptive type names instead of numeric OIDs.pgtap/withPoints/withPointsCost/types_check.pg (1)
63-71: Enhanced function signature validationThis change consistently follows the same pattern of improvement seen in other files, replacing OID-based type checking with more readable TEXT representations of PostgreSQL types.
tools/testers/tsp_pgtap_tests.sql (1)
187-189: Improved consistency in type checkingGood refactoring to use
function_types_eqfor clearer type validation, consistent with changes in other files.pgtap/traversal/binaryBreadthFirstSearch/types_check.pg (2)
67-75: Improved type checking with human-readable formatThe modification replaces OID-based type checking with human-readable TEXT arrays using the
function_types_eqfunction. This enhances code readability and maintainability by making the expected argument types explicit (e.g., 'text', 'int8', 'bool') rather than using numeric OIDs.
89-95: Consistent type definition approach in else branchThe same pattern of using
function_types_eqwith TEXT arrays has been correctly applied in the else branch as well, maintaining consistency throughout the conditional logic.pgtap/dijkstra/dijkstraNearCost/types_check.pg (1)
63-68: Converted to explicit type definitions using TEXT arraysThe code now uses
function_types_eqwith human-readable type names instead of numeric OIDs, improving readability and making the test more maintainable. The expected argument types are now clearly defined as text, anyarray, int8, etc.pgtap/components/strongComponenets/types_check.pg (1)
34-35: Improved type checking with human-readable formatThe change replaces OID-based type checking (previous implementation) with the more readable
function_types_eqfunction using explicit TEXT array. This makes the expected argument types forpgr_strongcomponentsclearer and easier to understand.pgtap/max_flow/maxFlowMinCost_Cost/types_check.pg (1)
21-21: Updated test plan countThe number of planned tests has been reduced from 14 to 13, which should correctly align with the actual number of tests being executed in this script.
pgtap/bdDijkstra/bdDijkstraCost/types_check.pg (2)
63-71: Improved type checking with human-readable formatThe changes replace the old OID-based type checking approach with a more readable TEXT array format using the
function_types_eqfunction. This improves code readability and maintenance by using explicit type names like 'text', 'int8', and 'bool' instead of numeric OIDs.
83-90: Consistent refactoring of type checking mechanismThis change maintains consistency with the earlier conditional block by using the same
function_types_eqapproach for earlier versions, improving maintainability while preserving the conditional logic.pgtap/bellman_ford/edwardMoore/types_check.pg (2)
67-74: Enhanced readability with function_types_eqThe code now uses the more intuitive
function_types_eqfunction with human-readable type names instead of querying OIDs directly from pg_proc. This makes the test more maintainable and easier to understand.
89-96: Consistent approach for version-dependent type checkingThe same improvement has been applied to the ELSE block for compatibility with earlier versions, ensuring a consistent approach throughout the conditional logic.
pgtap/utilities/findCloseEdges/types_check.pg (2)
58-64: Changed function_types to function_types_eqThe function call has been updated from
function_typestofunction_types_eq, which is consistent with the changes in other files, improving the standardization of type checking across the codebase.
77-81: Consistent type checking approach for older versionsThe same function update has been applied to the ELSE block, ensuring consistency in the type checking methodology regardless of version.
pgtap/ordering/topologicalSort/no_crash_test.pg (1)
1-77: Well-structured test for topological sort functionalityThis new test file is well-designed, testing the
pgr_topologicalsortfunction's existence, return types, error handling, and basic functionality. It follows the established testing patterns used throughout the pgtap suite.The test:
- Sets up appropriate test data in lines 25-33
- Verifies function existence and return types in lines 35-37
- Tests error handling with invalid parameters in lines 40-46
- Confirms the function works with valid inputs in lines 49-53
- Properly checks the return column types in lines 56-73
This comprehensive approach ensures the function behaves correctly and maintains compatibility with the expected interfaces.
pgtap/max_flow/edgedisjointpaths/types_check.pg (2)
60-66: Improved type checking with readable type names.The change replaces OID-based type checking with human-readable type names using the
function_types_eqfunction. This enhances code readability and maintainability.
80-85: Improved type checking in the fallback condition.Similar to the previous section, this change replaces OID-based type checking with human-readable type names in the ELSE block for versions before 3.2.0.
tools/testers/astar_pgtap_tests.sql (9)
6-6: Enhanced type variable naming for better readability.Changed
the_type_numbto use human-readable PostgreSQL type names ('float8') instead of OIDs, improving code clarity.Also applies to: 11-11
44-51: Improved type checking with descriptive type names.Changed from using
set_eqwith OID-based types tofunction_types_eqwith human-readable type names, making the code more maintainable.
64-71: Consistent type checking approach for earlier versions.Similar to the previous change, updated the type checking mechanism for PostgreSQL versions before 3.6.0 to use human-readable type names.
82-88: Standardized type checking for legacy versions.Updated the type checking approach for PostgreSQL versions before 3.2.0 to use the more readable text-based format.
99-99: Enhanced variable type naming in astarcost_types_check.Changed type representation from OIDs to human-readable PostgreSQL type names for better code clarity.
Also applies to: 104-104
137-144: Improved type checking in astarcost_types_check.Replaced OID-based type checking with human-readable type names using the
function_types_eqfunction.
155-160: Standardized type checking for earlier versions in astarcost_types_check.Consistent with other changes, updated the type checking mechanism for earlier PostgreSQL versions.
172-172: Enhanced variable type naming in astarcostmatrix_types_check.Changed from OIDs to human-readable PostgreSQL type names, improving code clarity.
Also applies to: 177-177
192-194: Improved type checking in astarcostmatrix_types_check.Updated the type checking approach to use human-readable type names.
tools/testers/flow_pgtap_tests.sql (2)
201-207: Improved type checking with human-readable type names.Replaced OID-based type checking with a more readable text-based format using the
function_types_eqfunction. This enhancement makes the code more maintainable.
221-226: Consistent type checking approach for earlier versions.Updated the type checking mechanism for earlier PostgreSQL versions to use the same improved format with human-readable type names.
pgtap/ksp/withPointsKSP/types_check.pg (2)
52-59: Improved type checking with descriptive type names.Replaced OID-based type checking with human-readable PostgreSQL type names using the
function_types_eqfunction. This change enhances code clarity and maintainability.
68-69: Standardized type checking for earlier versions.Consistent with the approach across other files, updated the type checking for earlier PostgreSQL versions to use human-readable type names.
tools/testers/spanningtree.sql (5)
14-16: Good refactoring to improve type checking readability.The change from using
set_eqwith PostgreSQL catalog queries tofunction_types_eqwith explicit TEXT arrays makes the code more readable and maintainable. Using human-readable type names (text,int8,float8) instead of OIDs improves clarity for future developers.
42-46: Improved type definition with human-readable types.The transition from OID-based type checking to explicit TEXT arrays makes the expected function signatures much clearer. This approach is more maintainable and less prone to errors when PostgreSQL internal OIDs change between versions.
58-62: Consistent refactoring pattern applied throughout the file.The changes maintain consistency with the version-specific checks while improving readability. Using explicit type arrays makes it easier to understand what argument types are expected by the function.
96-104: Well-structured type validation for multiple function overloads.The new approach handles multiple function signatures nicely with explicit type arrays. This implementation makes it much clearer what types are expected for each parameter compared to the previous OID-based approach.
116-124: Consistent approach across version-conditional branches.The same improvement pattern has been applied to both conditional branches, maintaining consistency throughout the file while improving readability and maintainability.
pgtap/withPoints/withPointsDD/types_check.pg (1)
58-64: Clean refactoring to more readable type checks.The change from
set_eqwith PostgreSQL catalog queries tofunction_types_eqwith explicit TEXT arrays improves clarity. Using human-readable types liketext,anyarray, andfloat8makes it easier to understand the function signatures.tools/testers/general_pgtap_tests.sql (2)
53-67: Improved function naming and implementation.The function has been renamed from what was likely
function_typestofunction_types_eq, which better reflects its purpose (it usesset_eqinternally). The implementation has been enhanced to:
- Use
coalesce(p.proallargtypes, p.proargtypes)which makes it more robust by handling cases whereproallargtypesmight be NULL- Properly reference the
pg_catalogschema for system tables, following best practices- Provide clearer context in the error message by including the function name
These changes improve both robustness and maintainability of the type checking system.
69-83: Good addition of a complementaryset_hasversion.Adding
function_types_hasthat usesset_hasinstead ofset_eqprovides more flexibility for type checking. This allows testing that a function has certain type signatures without requiring an exact match for all signatures, which can be useful in certain testing scenarios.The implementation correctly follows the same pattern as
function_types_eq, ensuring consistency across the codebase.pgtap/dijkstra/driving_distance/types_check.pg (2)
47-51: Improved readability with explicit type names.Replacing OID-based type checking with human-readable type names improves code clarity. The TEXT array
{text,int8,float8,bool,int8,int8,int8,int8,int8,int8,float8,float8}is much more meaningful than an equivalent OID array.
63-67: Consistent refactoring in version-specific logic.The changes maintain consistency with the version-specific checks while improving readability. Note that this branch uses
int4for the sequence ID rather thanint8in the newer version, which correctly preserves the function's evolution across versions.pgtap/max_flow/maxCardinalityMatch/types_check.pg (2)
43-47: Improved type checking readability through function_types_eqThe change from OID-based type checking to human-readable TEXT arrays is a good improvement for code maintainability. The new approach using
function_types_eqwith explicit type names (text, bool, int4, etc.) makes it much clearer what data types are expected for thepgr_maxcardinalitymatchfunction arguments.
56-57: Consistent use of function_types_eq in ELSE blockThe change to
function_types_eqin the ELSE block maintains consistency with the approach used in the first conditional block. Using TEXT arrays instead of OIDs improves readability and makes maintenance easier.pgtap/bellman_ford/bellman_ford/types_check.pg (2)
67-74: Enhanced type checking with explicit TEXT arraysThe replacement of OID arrays with explicit TEXT arrays in
function_types_eqsignificantly improves code readability. The new approach clearly indicates the expected argument types for thepgr_bellmanfordfunction, making it easier for developers to understand the function's interface.
89-95: Consistent type checking in ELSE blockThe ELSE block has been updated consistently with the same TEXT array approach. This standardized approach to type checking across different code paths is good practice and improves maintainability.
pgtap/contraction/contraction/types_check.pg (2)
39-41: Unified approach to function type checkingThe change from a previous check to
function_types_eqwith explicit TEXT arrays aligns this code with the new type-checking pattern being implemented across the codebase. Using human-readable type names enhances code clarity.
51-53: Consistent implementation in ELSE blockThe ELSE block has been updated to use the same
function_types_eqapproach, maintaining consistency throughout the conditional branches. This unified approach to type checking makes the code more maintainable.pgtap/dijkstra/dijkstraNear/types_check.pg (1)
63-69: Improved readability with explicit type specificationsThe change to
function_types_eqwith TEXT arrays significantly improves the readability of the type checking code. Using descriptive type names like 'text', 'bool', 'int8', etc. instead of OIDs makes it immediately clear what types are expected for each function parameter.pgtap/metrics/degree/types_check.pg (2)
51-55: Improved type checking withfunction_types_eqThe change replaces the previous OID-based type checking (
set_eqwith raw OIDs) with a more readable and maintainable approach usingfunction_types_eqwith human-readable type names. This makes the test more understandable and less prone to errors when maintaining the code.
63-64: Improved type checking for non-3.8.0 versionsSimilar to the previous change, this replaces the OID-based type checking with the more readable
function_types_eqapproach for versions before 3.8.0. The human-readable type names (text,text,bool,int8,int8) provide better clarity than the previous numeric OIDs.tools/testers/dijkstra_pgtap_tests.sql (3)
161-168: Enhanced type checking for pgr_dijkstra in version 3.5.0+The change replaces OID-based type checking with the more maintainable
function_types_eqfunction that uses human-readable type names. This significantly improves code readability and makes future maintenance easier, particularly when understanding function signatures.
187-194: Improved type checking for conditional version caseFor versions that match the condition
(min_version('3.2.0') AND fn != 'pgr_dijkstra') OR (min_version('3.1.0') AND fn = 'pgr_dijkstra'), the change replaces numeric OIDs with human-readable type names. This makes the test more understandable and consistent with the other changes.
206-211: Standardized type checking for earlier versionsFor earlier versions (the
ELSEblock), the change follows the same pattern of replacing OID-based type checking with human-readable type names through thefunction_types_eqfunction. This maintains consistency throughout the codebase and improves readability.
|
|
||
| SELECT function_types_eq('pgr_analyzegraph', | ||
| $$SELECT '{text,float8,text,text,text,text,text}'::TEXT[]$$ | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improved type checking with better readability.
The change to use function_types_eq with human-readable type names (text,float8,text,text,text,text,text) instead of OIDs enhances code clarity and consistency with other test files. This standardized approach makes the tests more maintainable.
Consider removing the extra blank line at line 33 for style consistency.
-
-
SELECT function_types_eq('pgr_analyzegraph',
$$SELECT '{text,float8,text,text,text,text,text}'::TEXT[]$$📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SELECT function_types_eq('pgr_analyzegraph', | |
| $$SELECT '{text,float8,text,text,text,text,text}'::TEXT[]$$ | |
| ); | |
| SELECT function_types_eq('pgr_analyzegraph', | |
| $$SELECT '{text,float8,text,text,text,text,text}'::TEXT[]$$ | |
| ); |
| SELECT function_types_eq(fn, | ||
| $$SELECT '{text,int8,int8,float823,23,23,float8float8float816,23,int8,float8,float8}'::TEXT[] $$ | ||
| ); |
There was a problem hiding this comment.
Fix missing separators in type array
The TEXT array containing type names appears to be missing proper separators between some data types.
- $$SELECT '{text,int8,int8,float823,23,23,float8float8float816,23,int8,float8,float8}'::TEXT[] $$
+ $$SELECT '{text,int8,int8,float8,23,23,23,float8,float8,float8,16,23,int8,float8,float8}'::TEXT[] $$The current format contains concatenated type names like float823 and float8float8float816 which should be separated with commas.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SELECT function_types_eq(fn, | |
| $$SELECT '{text,int8,int8,float823,23,23,float8float8float816,23,int8,float8,float8}'::TEXT[] $$ | |
| ); | |
| SELECT function_types_eq(fn, | |
| $$SELECT '{text,int8,int8,float8,23,23,23,float8,float8,float8,16,23,int8,float8,float8}'::TEXT[] $$ | |
| ); |
| function_types_eq('pgr_maxflow', | ||
| $$VALUES | ||
| ('{25,25}'::OID[]), | ||
| ('{25,20,20}'::OID[]), | ||
| ('{25,20,2277}'::OID[]), | ||
| ('{25,2277,20}'::OID[]), | ||
| ('{25,2277,2277}'::OID[]) | ||
| ('{text,text}'::TEXT[]), | ||
| ('{text,int8,int8}'), | ||
| ('{text,int8,anyarray}'), | ||
| ('{text,anyarray,int8}'), | ||
| ('{text,anyarray,anyarray}') | ||
| $$) |
There was a problem hiding this comment.
Function name mismatch in type check
The file appears to be testing pgr_maxflowmincost_cost based on the function checks at the top, but the function_types_eq call is checking pgr_maxflow instead. This inconsistency could cause the test to fail or produce incorrect results.
Correct the function name to match the function being tested:
- function_types_eq('pgr_maxflow',
+ function_types_eq('pgr_maxflowmincost_cost',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function_types_eq('pgr_maxflow', | |
| $$VALUES | |
| ('{25,25}'::OID[]), | |
| ('{25,20,20}'::OID[]), | |
| ('{25,20,2277}'::OID[]), | |
| ('{25,2277,20}'::OID[]), | |
| ('{25,2277,2277}'::OID[]) | |
| ('{text,text}'::TEXT[]), | |
| ('{text,int8,int8}'), | |
| ('{text,int8,anyarray}'), | |
| ('{text,anyarray,int8}'), | |
| ('{text,anyarray,anyarray}') | |
| $$) | |
| function_types_eq('pgr_maxflowmincost_cost', | |
| $$VALUES | |
| ('{text,text}'::TEXT[]), | |
| ('{text,int8,int8}'), | |
| ('{text,int8,anyarray}'), | |
| ('{text,anyarray,int8}'), | |
| ('{text,anyarray,anyarray}') | |
| $$) |
| function_types_eq('pgr_maxflow', | ||
| $$VALUES | ||
| ('{25,20,20}'::OID[]), | ||
| ('{25,20,2277}'::OID[]), | ||
| ('{25,2277,20}'::OID[]), | ||
| ('{25,2277,2277}'::OID[]) | ||
| ('{text,int8,int8}'::TEXT[]), | ||
| ('{text,int8,anyarray}'), | ||
| ('{text,anyarray,int8}'), | ||
| ('{text,anyarray,anyarray}') | ||
| $$) |
There was a problem hiding this comment.
Function name mismatch in ELSE branch
The same inconsistency exists in the ELSE branch. The function name should be pgr_maxflowmincost_cost to match the function being tested.
Correct the function name in the ELSE branch as well:
- function_types_eq('pgr_maxflow',
+ function_types_eq('pgr_maxflowmincost_cost',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function_types_eq('pgr_maxflow', | |
| $$VALUES | |
| ('{25,20,20}'::OID[]), | |
| ('{25,20,2277}'::OID[]), | |
| ('{25,2277,20}'::OID[]), | |
| ('{25,2277,2277}'::OID[]) | |
| ('{text,int8,int8}'::TEXT[]), | |
| ('{text,int8,anyarray}'), | |
| ('{text,anyarray,int8}'), | |
| ('{text,anyarray,anyarray}') | |
| $$) | |
| function_types_eq('pgr_maxflowmincost_cost', | |
| $$VALUES | |
| ('{text,int8,int8}'::TEXT[]), | |
| ('{text,int8,anyarray}'), | |
| ('{text,anyarray,int8}'), | |
| ('{text,anyarray,anyarray}') | |
| $$) |
| SELECT throws_ok( | ||
| 'SELECT * FROM pgr_topologicalsort( | ||
| ''SELECT id, source, target, cost, reverse_cost FROM edges id < 2'', | ||
| 3 | ||
| )','42883','function pgr_topologicalsort(unknown, integer) does not exist', | ||
| '6: Documentation says it does not work with 1 flags'); |
There was a problem hiding this comment.
Fix SQL syntax in error test
There's a syntax error in the SQL query used in the throws_ok test - missing WHERE keyword.
- ''SELECT id, source, target, cost, reverse_cost FROM edges id < 2'',
+ ''SELECT id, source, target, cost, reverse_cost FROM edges WHERE id < 2'',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SELECT throws_ok( | |
| 'SELECT * FROM pgr_topologicalsort( | |
| ''SELECT id, source, target, cost, reverse_cost FROM edges id < 2'', | |
| 3 | |
| )','42883','function pgr_topologicalsort(unknown, integer) does not exist', | |
| '6: Documentation says it does not work with 1 flags'); | |
| SELECT throws_ok( | |
| 'SELECT * FROM pgr_topologicalsort( | |
| ''SELECT id, source, target, cost, reverse_cost FROM edges WHERE id < 2'', | |
| 3 | |
| )','42883','function pgr_topologicalsort(unknown, integer) does not exist', | |
| '6: Documentation says it does not work with 1 flags'); |
| UPDATE edges SET cost = sign(cost), reverse_cost = sign(reverse_cost); | ||
| SELECT plan(5); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using a test-specific table instead of modifying the shared edges table
The code updates the shared edges table in line 22, which could potentially affect other tests running in the same session if they depend on specific values in the cost and reverse_cost columns.
Consider creating and using a dedicated test-specific table similar to how edges1 is created later, rather than modifying the shared table:
-UPDATE edges SET cost = sign(cost), reverse_cost = sign(reverse_cost);
+CREATE TABLE edges_topo AS SELECT id, source, target, sign(cost) as cost, sign(reverse_cost) as reverse_cost FROM edges;Then use this table in your test queries instead of the shared edges table.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UPDATE edges SET cost = sign(cost), reverse_cost = sign(reverse_cost); | |
| SELECT plan(5); | |
| CREATE TABLE edges_topo AS SELECT id, source, target, sign(cost) as cost, sign(reverse_cost) as reverse_cost FROM edges; | |
| SELECT plan(5); |
| function_types_eq('pgr_withpointsdd', | ||
| $$VALUES | ||
| ('{25,25,20,701,16,1042,16,23,20,20,701,701}'::OID[]), | ||
| ('{25,25,2277,701,16,1042,16,16,23,20,20,20,701,701}'::OID[]) | ||
| ('{text,text,anyarray,float8,bool,bpchar,bool,bool,int4,int8,int8,int8,float8,float8}'::TEXT[]), | ||
| ('{text,text,int8,float8,bool,bpchar,bool,int4,int8,int8,float8,float8}') | ||
| $$) |
There was a problem hiding this comment.
Missing type cast in function signature array.
Line 77 is missing the ::TEXT[] cast that's present in all other similar type definitions. This could cause the test to fail.
-('{text,text,int8,float8,bool,bpchar,bool,int4,int8,int8,float8,float8}')
+('{text,text,int8,float8,bool,bpchar,bool,int4,int8,int8,float8,float8}'::TEXT[])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function_types_eq('pgr_withpointsdd', | |
| $$VALUES | |
| ('{25,25,20,701,16,1042,16,23,20,20,701,701}'::OID[]), | |
| ('{25,25,2277,701,16,1042,16,16,23,20,20,20,701,701}'::OID[]) | |
| ('{text,text,anyarray,float8,bool,bpchar,bool,bool,int4,int8,int8,int8,float8,float8}'::TEXT[]), | |
| ('{text,text,int8,float8,bool,bpchar,bool,int4,int8,int8,float8,float8}') | |
| $$) | |
| function_types_eq('pgr_withpointsdd', | |
| $$VALUES | |
| ('{text,text,anyarray,float8,bool,bpchar,bool,bool,int4,int8,int8,int8,float8,float8}'::TEXT[]), | |
| ('{text,text,int8,float8,bool,bpchar,bool,int4,int8,int8,float8,float8}'::TEXT[]) | |
| $$) |
Demo
@pgRouting/admins
Summary by CodeRabbit