Issue 2807 wrap proallargtypes test functions #2812
Conversation
WalkthroughThis PR updates the mechanism for validating function argument types in a range of SQL test files. The previous approach—querying PostgreSQL’s catalog and comparing OID arrays using functions like Changes
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 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: 10
📜 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/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(1 hunks)pgtap/max_flow/maxFlowMinCost/types_check.pg(2 hunks)pgtap/max_flow/maxFlowMinCost_Cost/types_check.pg(1 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/types_check.pg(1 hunks)pgtap/others/alpha_shape/types_check.pg(1 hunks)pgtap/others/dagShortestPath/types_check.pg(3 hunks)pgtap/others/transitiveClosure/types_check.pg(1 hunks)pgtap/pickDeliver/pickDeliver/types_check.pg(1 hunks)pgtap/pickDeliver/pickDeliverEuclidean/types_check.pg(1 hunks)pgtap/pickDeliver/vrp_basic/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/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/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)tools/testers/types_check.sql(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: macos (macos-latest)
- GitHub Check: Ubuntu Boost (77)
- GitHub Check: Ubuntu Boost (80)
- GitHub Check: Ubuntu Boost (86)
- GitHub Check: Ubuntu Boost (78)
- GitHub Check: macos (macos-14)
- GitHub Check: Ubuntu Boost (79)
- GitHub Check: Ubuntu Boost (83)
- GitHub Check: Ubuntu Boost (68)
- GitHub Check: Ubuntu Boost (56)
- GitHub Check: Ubuntu Boost (84)
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: Ubuntu Boost (76)
- GitHub Check: Check queries
- GitHub Check: Ubuntu clang
🔇 Additional comments (109)
pgtap/topology/analyzeGraph/types_check.pg (1)
33-36: Improved type checking implementationThe change replaces OID-based type checking with a clearer text-based approach using
function_types_eq. This enhances code readability and maintainability by making the expected argument types explicitly visible in the test code.pgtap/components/bridges/types_check.pg (1)
34-35: Improved type checking approach.The change from using OID-based comparisons to explicit type names with
function_types_eqenhances readability and maintainability. Using text-based type names ('text,int8') instead of numeric OIDs makes the test more self-documenting and easier to understand.pgtap/dominator/lengauerTarjanDominatorTree/types_check.pg (1)
47-51: Improved type checking approachThe change from using
set_eq()with OID arrays tofunction_types_eq()with text-based type names is a good improvement. This makes the code more readable and maintainable by:
- Eliminating the need for a subquery to fetch
proallargtypesfrompg_proc- Using human-readable type names ('text', 'int8', 'int4') instead of numeric OIDs
- Simplifying the test structure with a direct function call
pgtap/version/full_version/types_check.pg (1)
37-38: Improved type checking approachThe change from using PostgreSQL catalog queries with
set_eqto the more directfunction_types_eqis a positive improvement. Using explicit text type names instead of OIDs makes the tests more readable and maintainable, as type names are self-documenting while OIDs are opaque numeric values.This change aligns with the broader effort mentioned in the PR summary to update the mechanism for validating function argument types across test files.
pgtap/traversal/breadthFirstSearch/types_check.pg (1)
41-45: Improved type checking with more readable function_types_eqThe change from using raw
set_eqwith a query againstpg_procto the more specializedfunction_types_eqfunction is a good improvement. This refactoring makes the type checking more readable and maintainable by:
- Using explicit text-based type names (like
text,int8,bool,float8) instead of numeric OIDs- Encapsulating the type checking logic in a dedicated function
- Making the test more intention-revealing
This change is consistent with the overall PR objective of wrapping
proallargtypestest functions.pgtap/others/alpha_shape/types_check.pg (1)
26-28: Good improvement to type checking implementationThis change simplifies the function argument type validation by replacing a more complex approach (likely using OIDs and CTEs) with a direct call to
function_types_eq. Using text-based type names (geometryandfloat8) instead of numeric OIDs makes the test more readable and maintainable.The improved approach is consistent with the broader changes mentioned in the PR summary, standardizing type checking across multiple functions with a more direct and readable method.
pgtap/components/strongComponenets/types_check.pg (1)
34-35: Good improvement to type checking approach.The change from using OID-based type comparison to text-based representation is a significant improvement. This makes the tests more readable and maintainable as text type names (
text,int8) are more self-explanatory than numeric OIDs. The use offunction_types_eqis also more appropriate for this specific testing purpose than the genericset_eqfunction.pgtap/version/version/types_check.pg (1)
24-24: Changes to test plan align with simplified testing strategy.The reduction from 6 to 3 planned tests is consistent with the PR's goal of streamlining function argument type validation. According to the AI summary, three
SELECT set_eqstatements were removed that previously checked the function's argument names, argument types, and return type separately. The remaining tests still validate that:
- The
pgr_versionfunction exists- It accepts no arguments
- It returns text
This approach is more maintainable while preserving all necessary validation.
pgtap/dijkstra/dijkstraNear/types_check.pg (1)
63-68: Improved type checking with more readable type definitions.This change replaces the OID-based type checking with a more explicit text-based approach using
function_types_eq. The new implementation is more readable and maintainable as it uses human-readable type names rather than numeric OIDs.The conversion from OID-based types to text-based types also makes the test more robust against potential changes in PostgreSQL's internal type OIDs across different versions.
pgtap/ksp/turnRestrictedPath/types_check.pg (1)
34-36: Great improvement to type checking implementation!This change enhances readability and maintainability by:
- Replacing numeric OIDs with human-readable type names (text, int8, bool, etc.)
- Using the specialized
function_types_eqfunction which encapsulates the type comparison logic- Making the test more straightforward and easier to understand
The updated implementation aligns with modern best practices and the broader effort to standardize type checking across the codebase.
pgtap/components/biconnectedComponents/types_check.pg (1)
34-35: Improved type checking approach.The change from using
set_eqwith apg_procquery to usingfunction_types_eqwith explicit type names is a good improvement. This approach is:
- More readable - using text representation of types (
text,int8,int8,int8) is easier to understand than OIDs- More maintainable - directly specifies the expected types rather than querying the catalog
- More consistent - aligns with the standardization effort mentioned in the PR summary
pgtap/topology/extractVertices/types_check.pg (1)
33-34: Improved function argument type checking methodologyThe change replaces the previous OID-based type checking with a more readable and maintainable text-based approach using
function_types_eq. This aligns with the PR's objective of wrapping proallargtypes test functions and makes the type validation more explicit.The new approach directly specifies the expected argument types (
text,bool,int8,_int8,_int8,float8,float8,geometry) which correspond to the function parameters shown in the proargnames test on line 30 ("","dryrun","id","in_edges","out_edges","x","y","geom").pgtap/metrics/betweennessCentrality/types_check.pg (1)
50-53: Improved type checking with function_types_eq is a good change.This change replaces the direct SQL query to PostgreSQL's catalog table with the more readable and maintainable
function_types_eqfunction. The switch from OID-based type representation to text-based type names (text,bool,int8,float8) makes the tests more human-readable and easier to maintain. This aligns with the broader approach being taken across the codebase as described in the PR summary.pgtap/topology/createTopology/types_check.pg (1)
33-34: Improved type checking approach.The change replaces the previous catalog query and OID comparison with a direct call to
function_types_equsing text-based type names. This approach is more readable and maintainable than using numeric OIDs, making the test easier to understand at a glance.The new type array correctly matches the function signature defined in line 25, with appropriate type mappings (
float8fordouble precisionandboolforboolean).pgtap/topology/analyzeOneWay/types_check.pg (1)
33-34: Improved type checking approach.This change enhances code maintainability by replacing OID-based type checking with a more readable and descriptive text-based approach. Using
function_types_eqwith explicit type names liketextandboolmakes the tests more intuitive and less dependent on PostgreSQL's internal OID representations, which can vary between versions.The new approach is consistent with similar changes across other functions in the codebase, representing a positive modernization effort.
pgtap/traversal/binaryBreadthFirstSearch/types_check.pg (2)
67-74: Good improvement to readability with explicit type namesThe transition from OID arrays to more descriptive TEXT arrays for type checking functions makes the code significantly more readable and maintainable. Using explicit type names like
int8andfloat8instead of numeric OIDs provides better clarity about what types are expected.The
function_types_eqfunction is also a more direct approach than the previous method, streamlining the type verification process.
89-95: Consistent implementation for backward compatibilityGood job applying the same pattern to the pre-3.2.0 version check. The consistent approach across version conditionals enhances maintainability while ensuring backward compatibility.
pgtap/lineGraph/lineGraph/types_check.pg (1)
33-34: Improved type checking with human-readable type namesThis change enhances maintainability by replacing the previous OID-based type checking with a direct call to
function_types_equsing human-readable type names. The textual representation (text,bool,int4, etc.) is much more intuitive and self-documenting than numeric OID values.This aligns well with the PR objective to update the type validation mechanism across the codebase.
pgtap/pickDeliver/vrp_basic/types_check.pg (1)
32-33: Improved type checking approachThe change from using
set_eqwith catalog queries tofunction_types_eqwith explicit text arrays is a good improvement. This makes the tests more readable and maintainable by:
- Using explicit type names (text, int4) instead of numeric OIDs
- Simplifying the test structure with a dedicated function
- Following a consistent pattern across the codebase
This aligns well with the PR objective of wrapping proallargtypes test functions.
pgtap/ksp/withPointsKSP/types_check.pg (2)
51-60: Improved readability with function_types_eq and explicit type namesThe replacement of
set_eqwithfunction_types_eqenhances code clarity. Using explicit type names (like 'int8', 'text', 'bool') instead of PostgreSQL OIDs makes the test more readable and maintainable.
67-70: Consistent approach for older versionsGood consistency in also updating the ELSE clause with the same pattern. The change maintains the conditional logic based on PostgreSQL version while improving readability with explicit type names.
pgtap/topology/createVerticesTable/types_check.pg (1)
33-34: Improved type checking implementation.The change from direct SQL querying of OIDs to using
function_types_eqwith text-based type names is a positive improvement. This approach is:
- More readable (text types vs. numeric OIDs)
- More maintainable (using a dedicated testing function)
- Consistent with changes across the codebase mentioned in the PR summary
The test still verifies the same functionality - that
pgr_createverticestableaccepts five text arguments - but does so in a cleaner, more modern way.pgtap/pickDeliver/pickDeliver/types_check.pg (1)
36-39:Details
❓ Verification inconclusive
Improved code clarity with function_types_eq
The change from using
set_eqwith PostgreSQL catalog queries to usingfunction_types_eqimproves readability and maintainability. The explicit text-based type specification (text,float8,int4, etc.) is more human-readable than numeric OIDs.However, please verify that the array of 19 types accurately represents both the function's input parameters (6 parameters defined on lines 24-25) and output columns (13 output columns listed on lines 32-34). The count suggests there might be additional types being checked.
🏁 Script executed:
#!/bin/bash # Verify the function signature of pgr_pickdeliver psql -c "\df pgr_pickdeliver" -d postgresLength of output: 138
Manual Verification Required: Confirm Function Signature Alignment
The switch to
function_types_eqis a clear improvement in readability and maintainability. However, the verification that the 19-element type array accurately reflects the 6 input parameters (as per lines 24–25) and the 13 output columns (as per lines 32–34) remains inconclusive. The initial psql verification script failed (i.e., thepsqlcommand was not found), so please run this verification manually in an environment where PostgreSQL is available, or use an alternative method to extract and confirm the function signature.
- Action Required: Manually verify that the type array (19 types) precisely corresponds to the function's 6 input parameters and 13 output columns.
tools/testers/flow_pgtap_tests.sql (2)
201-208:Details
❓ Verification inconclusive
Use of
function_types_eqfor Version 3.2.0+ ChecksThe new call to
function_types_eqreplaces the old OID-based type validation by comparing explicitly defined textual type arrays. This improves clarity by directly specifying the expected type names. Please verify that the five arrays listed correctly reflect the updated function signatures for the newer version. Additionally, consider whether the coexistingset_eqcall (in line 191–200) is still necessary or if it can be deprecated in favor of the new approach.
Confirm Updated Type Array Validation and set_eq Deprecation
The newfunction_types_eqcall now uses explicit textual type arrays in place of the old OID-based checks, which should make the expected signatures for version 3.2.0+ clearer. Please verify that each of the five arrays in lines 201–208 accurately reflects the updated function signatures. In addition, review the existingset_eqcall (lines 191–200) to determine whether it is now redundant and can safely be removed.
221-227:Details
❓ Verification inconclusive
Ensure Consistency in Textual Type Arrays for Older Versions
In the ELSE branch, the added
function_types_eqcall now validates four textual type arrays. Confirm that these arrays accurately represent the expected argument types for legacy scenarios. The discrepancy in the number of arrays between the version branches appears intentional; however, adding an inline comment explaining this version-specific behavior would enhance clarity for future maintainers.
Action: Verify Legacy Type Arrays and Add Inline Documentation
- In file tools/testers/flow_pgtap_tests.sql (lines 221–227), the
function_types_eqcall now checks four textual type arrays.- Please confirm that these arrays accurately capture the expected argument types for legacy scenarios.
- Additionally, add an inline comment near this function call to explain the intentional discrepancy in the number of arrays between version branches, which will aid future maintainers in understanding the version-specific behavior.
pgtap/withPoints/withPointsDD/types_check.pg (2)
57-64: Well-structured refactoring of type checking mechanism.The change from using
set_eqwith a query to using the more directfunction_types_eqis a good improvement that makes the test cleaner and more maintainable. The use of explicit type names like 'text', 'anyarray', 'float8' instead of numeric OIDs increases readability.
45-82: Consider addressing the TODO comments.There are two TODO comments indicating code that should be removed in version 4:
- "TODO remove last 2 rows on v4" (line 45)
- "TODO remove on v4 this are the old signatures" (line 82)
These comments suggest that some of this code is being maintained for backward compatibility but will be removed in a future version. Consider whether this PR is an appropriate time to address these TODOs if version 4 is imminent or already released.
pgtap/allpairs/floydWarshall/types_check.pg (1)
34-37: Improved type checking with function_types_eqThe change from using
set_eqwith a direct SQL query tofunction_types_eqis a good improvement. This approach makes the code more maintainable by:
- Using more readable text-based type representations instead of numeric OIDs
- Providing a more direct and purpose-specific function for type checking
- Making the intent clearer to developers reading the code
pgtap/ordering/cuthillMckeeOrdering/types_check.pg (1)
46-49: Better type checking approachThe replacement of
bag_eqwithfunction_types_eqfollows the same pattern of improvement seen in other files. This change enhances readability by using text type names (text,int8,int8) instead of cryptic OIDs. The function namefunction_types_eqalso better communicates the intent of this test.pgtap/contraction/contractionDeadEnd/types_check.pg (1)
43-44: Consistent use of function_types_eqThis change replaces
function_typeswithfunction_types_eqwhile maintaining the same text-based type representation. This standardizes the approach to type checking across the codebase, making the API more consistent.pgtap/trsp/trspViaVertices/types_check.pg (1)
37-40: Improved readability with explicit type namesThe change from using
set_eqwith OIDs tofunction_types_eqwith text type names significantly improves code readability. The new representation makes it immediately clear what types are expected (text,anyarray,bool,bool,text,int4,int4,int4,int4,float8), whereas the previous OID values (25,2277,16,16,25,23,23,23,23,701) required looking up the mapping between OIDs and types.pgtap/others/transitiveClosure/types_check.pg (1)
40-44: Improved type checking with function_types_eqThe change from using a PostgreSQL catalog query with
set_eqto using the more specializedfunction_types_eqfunction is a good improvement. This makes the test more explicit about the expected argument types forpgr_transitiveclosureand moves from numeric OIDs to human-readable type names (text,int4,int8,_int8).The new approach is more maintainable as it clearly documents the expected types in the test itself rather than relying on dynamic catalog queries.
pgtap/ordering/topologicalSort/types_check.pg (1)
40-44: Improved type checking with function_types_eqThe change from using a PostgreSQL catalog query with
set_eqto using the more specializedfunction_types_eqfunction enhances the clarity of the test. This approach explicitly specifies the expected argument types forpgr_topologicalsortas text values (text,int4,int8) rather than using numeric OIDs.This modification improves maintainability by making the test more self-documenting regarding the expected function signature.
pgtap/components/makeConnected/types_check.pg (1)
45-49: Improved type checking with function_types_eqGood change replacing the catalog query and
set_eqwithfunction_types_eq. The explicit specification of argument types using readable names (text,int8,int8,int8) improves test clarity.Note that the test still uses
set_eqin lines 39-42 to check the argument names. This is consistent with the pattern of separating type checking from name checking.pgtap/lineGraph/lineGraphFull/types_check.pg (1)
33-35: Improved type checking with function_types_eqThe change to use
function_types_eqimproves the test by explicitly specifying the expected argument types as human-readable names (text,int4,int8,int8,float8,int8). This is more maintainable than using OIDs.Note the slight syntax difference compared to other files - here the TEXT array is passed directly with a SELECT statement rather than using the VALUES clause, but the functionality is equivalent.
pgtap/trsp/trspViaEdges/types_check.pg (1)
38-40: Improved type checking implementationThe change replaces direct PostgreSQL catalog queries and OID array comparisons with the more readable
function_types_eqfunction. Using text-based type names (text,_int4, etc.) instead of numeric OIDs makes the test more maintainable and easier to understand.pgtap/coloring/sequentialVertexColoring/types_check.pg (1)
46-48: Improved type checking approachGood change that replaces the previous catalog query using
set_eqwith a direct function call tofunction_types_eq. Using explicit type names instead of numeric OIDs enhances readability and maintainability of the test code.pgtap/coloring/edgeColoring/types_check.pg (1)
46-48: Improved type checking with readable type namesGood change that replaces the PostgreSQL catalog query and OID comparison with a direct function call to
function_types_eq. Using explicit type names (text,int8) instead of numeric OIDs improves code readability and maintainability.pgtap/allpairs/johnson/types_check.pg (1)
34-37: Improved type checking with function_types_eqThe change from using a PostgreSQL catalog query to directly using
function_types_eqis an improvement. The new approach simplifies the code by eliminating the subquery and directly comparing against human-readable type names (text,bool,int8,int8,float8) instead of OIDs, making the code more maintainable.pgtap/withPoints/withPointsCostMatrix/types_check.pg (1)
34-37: Type checking simplified with function_types_eqThe replacement of the catalog query with
function_types_eqmakes the type checking more straightforward. Using text representations of data types (text,text,anyarray,bool,bpchar,int8,int8,float8) instead of numeric OIDs improves readability and maintainability of the test code.pgtap/contraction/contractionLinear/types_check.pg (1)
43-44: Improved type checking with function_types_eqThe change from using
set_eqwithproallargtypesto directly usingfunction_types_eqis a good improvement. This approach is more direct and readable, using explicit text-based type names instead of numeric OIDs.pgtap/coloring/bipartite/types_check.pg (1)
45-49: Good refactoring to function_types_eqReplacing
set_eqwithfunction_types_eqsimplifies the code and improves readability. Using explicit text-based type names ({text,int8,int8}) instead of numeric OIDs makes the code more maintainable and easier to understand.pgtap/others/dagShortestPath/types_check.pg (2)
65-73: Good refactoring to function_types_eq in the first code pathThe change to use
function_types_eqwith explicit text-based type names makes the code more readable and maintainable for the version 3.2.0+ code path.
88-95: Good refactoring to function_types_eq in the alternative code pathThe consistent update to use
function_types_eqwith explicit text-based type names in the pre-3.2.0 code path maintains consistency across version conditions.pgtap/components/articulationPoints/types_check.pg (1)
34-36: Code looks good - improved type checking methodThe change replaces an implicit OID-based type comparison with a more explicit text-based type check using
function_types_eq. This improves readability and maintainability by using human-readable type names ('text', 'int8') instead of numeric OIDs.pgtap/utilities/findCloseEdges/types_check.pg (2)
57-64: Function type checking method improvedGood replacement of
function_typeswithfunction_types_eqfor checking function argument types. This provides a more explicit equality check and is consistent with the changes being made across the codebase for type verification.
77-81: Consistent type checking improvementThe change to
function_types_eqin the else clause is consistent with the earlier change, maintaining the same pattern throughout both conditional branches. This ensures consistent behavior regardless of which PostgreSQL version is being used.pgtap/bdDijkstra/bdDijkstraCost/types_check.pg (2)
64-71: Enhanced type checking with improved readabilityGood replacement of OID-based type checking with more readable text representations. Using
function_types_eqwith explicit type names like 'text', 'int8', and 'float8' makes the code more maintainable and easier to understand than using numeric OIDs.
84-90: Consistent type checking approachThe consistent application of
function_types_eqin this else branch maintains the same pattern throughout the conditional logic. The explicit type names improve code clarity and maintainability.pgtap/topology/nodeNetwork/types_check.pg (1)
33-35: Improved type checking with better readabilityGood replacement of
set_eqon OIDs with the more explicitfunction_types_equsing text representations. This change enhances readability and maintainability by using descriptive type names ('text', 'float8', 'bool') instead of numeric OIDs.pgtap/bdDijkstra/bdDijkstraCostMatrix/types_check.pg (1)
34-37: Improved type checking with function_types_eqThis change enhances readability by replacing a direct catalog query with the more descriptive
function_types_eqfunction. The explicit TEXT array representation of parameter types makes the test more self-documenting and easier to maintain.pgtap/circuits/hawickCircuits/types_check.pg (1)
50-53: Improved type validation approachThe change to
function_types_eqwith explicit type names is a good improvement over queryingpg_procdirectly. Using descriptive TEXT array types (text,int4,int8, etc.) instead of numeric OIDs makes the test more readable and maintainable.pgtap/ksp/ksp/types_check.pg (2)
54-61: Enhanced type checking with explicit type namesThis refactoring improves code clarity by using the specialized
function_types_eqfunction with explicit type names rather than OID values. The TEXT array representation with PostgreSQL type names is much more readable and maintainable.
70-72: Consistent approach for older versionsGood consistency by applying the same approach to the version-specific check. Using
function_types_eqwith type names in TEXT format maintains a uniform testing pattern throughout the codebase.pgtap/traversal/depthFirstSearch/types_check.pg (1)
51-56: Improved type validation with textual type representationSwitching from
set_eqwith catalog queries tofunction_types_eqwith explicit type arrays provides better readability and maintainability. The TEXT representation of types makes it immediately clear what types are expected for each function parameter.pgtap/dijkstra/dijkstraCostMatrix/types_check.pg (1)
34-37: Looks good - Enhanced type checking approachThe change replaces the direct query to pg_proc with a cleaner function_types_eq() call. This improves readability by using explicit type names (TEXT) instead of OIDs, making it easier to understand and maintain the type validation.
pgtap/max_flow/maxFlow/types_check.pg (2)
57-66: Improved type checking with better readabilityNice improvement in the min_version('3.2.0') branch. The change from OID arrays to TEXT representations of types ('{text,text}', '{text,int8,int8}', etc.) makes the expected function signatures much clearer to understand and maintain.
68-75: LGTM - Consistent type representation in the ELSE branchThe ELSE branch has been updated with the same cleaner approach using TEXT representations instead of OIDs, maintaining consistency with the changes throughout the codebase.
pgtap/withPoints/withPoints/types_check.pg (2)
63-70: Good enhancement to type checkingThe change from set_eq with OID arrays to function_types_eq with explicit TEXT type arrays significantly improves readability. The new format clearly communicates the expected parameter types for pgr_withpoints.
83-89: Appropriate update to the ELSE branchThe ELSE branch has been consistently updated with the same TEXT-based type checking approach, properly handling the version-specific differences in the function signature.
pgtap/max_flow/edgedisjointpaths/types_check.pg (2)
60-67: Good conversion from OID-based to text-based type checking.The change to use
function_types_eqwith text-based type representations improves code readability and maintainability. This approach makes the type checking more explicit and easier to understand.
80-86: Consistent implementation in the ELSE branch.The same improvement has been correctly applied to the ELSE branch of the conditional, maintaining consistency throughout the code.
pgtap/withPoints/withPointsCost/types_check.pg (2)
64-71: Improved type checking with explicit type names.Good refactoring from OID arrays to text-based type representations, which makes the code much more readable and maintainable. The explicit type names ('text', 'int8', etc.) are much clearer than numeric OIDs.
84-90: Consistent implementation in the ELSE branch.The changes are correctly and consistently applied to the ELSE branch as well, ensuring uniform behavior across different PostgreSQL versions.
pgtap/max_flow/maxCardinalityMatch/types_check.pg (2)
43-47: Well-implemented type checking refactoring.The change to use
function_types_eqwith explicit text type arrays is a good improvement. This makes the type checking more readable and easier to maintain than using OIDs.
56-58: Consistent implementation in the ELSE clause.The ELSE branch also correctly implements the new approach with
function_types_eq, maintaining consistency throughout the codebase.tools/testers/spanningtree.sql (5)
14-15: Improvement: Updated type checking mechanismThe switch from using
set_eqwith OID arrays tofunction_types_eqwith TEXT arrays improves readability and maintainability. Text-based type names liketext,int8, andfloat8are much clearer than numeric OIDs.
42-46: Enhanced type validation approachGood refactoring to use
function_types_eqwith TEXT arrays instead of OID-based checks. This makes the type specifications more explicit and human-readable, while maintaining the same validation logic.
58-62: Consistent improvement in type checkingThis change follows the same pattern as the previous one, replacing OID arrays with TEXT arrays for better readability while preserving the validation logic.
96-104: Improved readability for complex type definitionsThe change from OID arrays to TEXT arrays significantly improves the readability of these complex type definitions, making them easier to maintain and understand.
116-124: Consistent type definition improvementsThis change maintains consistency with the earlier modifications, enhancing readability while preserving the same validation logic. The TEXT-based type names make it much clearer what each parameter represents.
pgtap/metrics/degree/types_check.pg (2)
51-55: Improved type checking approachGood change replacing the OID-based checks with more readable TEXT arrays through
function_types_eq. This makes the type specifications much clearer and easier to understand.
63-64: Improved type validation in else branchThis change consistently applies the new type checking approach in the else branch as well, making the entire function more uniform and maintainable.
tools/testers/general_pgtap_tests.sql (2)
53-67: Good function renaming and implementation enhancementThe function has been appropriately renamed from
function_typestofunction_types_eqto better reflect its purpose. The implementation has been improved with:
- Using
coalesce(p.proallargtypes, p.proargtypes)to handle cases whereproallargtypesmight be null- Explicitly referencing the catalog schemas (
pg_catalog.pg_procandpg_catalog.pg_type) for clarityThese changes make the function more robust and maintainable.
69-83: Well-implemented new helper functionThe new
function_types_hasfunction is a good addition that follows the same pattern asfunction_types_eqbut usesset_hasinstead ofset_eq. This provides flexibility for checking whether a set contains specific function types without requiring an exact match.pgtap/dijkstra/dijkstraNearCost/types_check.pg (1)
63-69: Improved readability of complex type specificationsGood change to use
function_types_eqwith TEXT arrays instead of OID-based type checks. This significantly improves readability and maintainability, especially for these complex type definitions with multiple function signatures.pgtap/max_flow/maxFlowMinCost/types_check.pg (2)
63-70: Improved type checking implementation using clearer type representationsThe change from
set_eqtofunction_types_eqwith explicit text-based type representations is a good improvement. This makes the tests more readable and maintainable compared to the previous OID-based approach.
83-89: Consistent implementation in the else clauseThe same improvement has been made in the else clause for older versions, maintaining consistency across different PostgreSQL versions.
pgtap/contraction/contraction/types_check.pg (2)
39-41: Appropriate use of function_types_eq for improved readabilityReplacing the previous implementation with
function_types_eqprovides better readability and consistency with other test files in the codebase.
51-53: Consistent implementation for older PostgreSQL versionsThe change to use
function_types_eqwith explicit type names makes the type checking more transparent and easier to maintain compared to queryingproallargtypesdirectly.pgtap/dijkstra/driving_distance/types_check.pg (2)
47-51: Standardized approach for type checkingThe switch to
function_types_eqwith explicit TEXT array type definitions improves code clarity and follows the standardized approach being implemented across the codebase.
63-67: Consistent implementation for older versionsThe type checking approach is consistently applied for older versions as well, ensuring uniform test behavior across different PostgreSQL versions.
pgtap/dijkstra/dijkstraCost/types_check.pg (2)
63-70: Enhanced type checking with clearer type representationsConverting from OID-based type checking to explicit text-based type representations makes the tests more readable and maintainable. The use of
function_types_eqaligns with the standardized approach across the codebase.
82-88: Consistent implementation for backward compatibilityThe same improved approach is applied for older PostgreSQL versions, maintaining consistency in the codebase while ensuring backward compatibility.
tools/testers/astar_pgtap_tests.sql (9)
6-6: Code improvement: Enhanced type representationThe change from numeric OIDs to descriptive text-based type names ('float8' and 'numeric') improves code readability and makes the type declarations more explicit.
Also applies to: 11-11
44-51: Good refactoring: Improved type checking approachReplacing the
set_eqquery withfunction_types_eqand using textual type representations instead of OIDs enhances readability and maintainability of the code. The new approach is more direct and easier to understand.
64-71: Consistent approach to type checkingThis change follows the same pattern as the previous section, consistently applying the
function_types_eqapproach with textual type names. This consistency helps maintain code clarity throughout the file.
82-88: Consistent implementation of type checkingThe changes in this section maintain the same improved approach to type checking, ensuring consistent implementation throughout the function.
99-99: Improved type representation for astarcost_types_check functionSimilar to the previous function, the type representations have been updated to use text-based names instead of OIDs for better readability.
Also applies to: 104-104
137-144: Enhanced type checking in astarcost_types_checkThe implementation of
function_types_eqwith textual type names consistently follows the pattern established earlier in the file, making the type checking more readable and maintainable.
155-160: Consistent approach in alternate conditionThe consistent application of the improved type checking methodology across different conditions ensures code uniformity and maintainability.
172-172: Improved type representation in astarcostmatrix_types_checkUpdated the type representation to use 'float8' and 'numeric' text-based names instead of OIDs, consistent with the changes in other functions.
Also applies to: 177-177
192-195: Enhanced type checking for astarcostmatrix_types_checkThe implementation of
function_types_eqwith textual type names maintains consistency with other functions, improving readability and maintainability.tools/testers/types_check.sql (8)
7-7: Improved variable type declarationsChanged variables from OID arrays to TEXT arrays with more descriptive type names, making the code more readable and the data types more explicit. This change enhances code clarity and maintainability.
Also applies to: 11-11, 14-14, 16-16, 18-18
36-37: Consistent type representation in array operationsUpdated array concatenation operations to use TEXT arrays with descriptive type names, maintaining consistency with the variable type changes. This ensures all type references throughout the function use the same improved format.
Also applies to: 43-44, 51-52, 54-55
107-125: Enhanced type checking with function_types_eqReplaced
set_eqwithfunction_types_eqand updated format strings to work with TEXT arrays rather than OIDs. This change improves code readability and standardizes the approach to type checking.
139-144: Consistent type checking for combinationsApplied the same improvements to the combinations condition block, maintaining consistency in the type checking approach throughout the function.
164-186: Improved type checking with function_types_hasReplaced the previous implementation with
function_types_hasand updated the format to work with TEXT arrays. This change is part of the broader standardization of type checking methods and improves code readability.
199-199: Enhanced variable declarations in types_check_viaUpdated variable declarations in the
types_check_viafunction to use TEXT arrays instead of OID arrays, consistent with the changes in thetypes_check_generalfunction.Also applies to: 203-203, 206-206
218-218: Consistent type representation in array operationsUpdated array concatenation operations in the
types_check_viafunction to use TEXT representations, maintaining consistency with the variable type changes.Also applies to: 225-225, 229-229, 235-235
251-257: Enhanced type checking in types_check_viaReplaced
set_eqwithfunction_types_eqfor type checking in thetypes_check_viafunction, consistent with the changes in thetypes_check_generalfunction. This standardizes the approach to type checking across functions.pgtap/chinese/chinesePostman/types_check.pg (1)
34-35: Improved type checking for pgr_chinesepostmanReplaced
set_eqwithfunction_types_eqand updated the type representation from OIDs to text-based types. This change improves code readability and is consistent with similar changes in other files.pgtap/chinese/chinesePostmanCost/types_check.pg (1)
39-41: Enhanced type checking for pgr_chinesepostmancostReplaced
set_eqwithfunction_types_eqand updated the type representation to use a TEXT array. This change aligns with the standardization of type checking across the codebase, improving readability and maintainability.pgtap/bellman_ford/bellman_ford/types_check.pg (2)
67-74: Improved type checking with function_types_eqThe replacement of direct catalog queries with
function_types_eqis a good improvement. Using human-readable type names (e.g., 'text', 'int8', 'bool') instead of OIDs makes the test more maintainable and easier to understand.
89-94: Consistent improvement to type checkingSimilar to the previous change, this section benefits from using
function_types_eqwith human-readable type names instead of directly querying the catalog with OIDs. This maintains consistency in the approach to type checking throughout the function.pgtap/bellman_ford/edwardMoore/types_check.pg (2)
67-74: Enhanced type checking with function_types_eqUsing
function_types_eqwith explicit type names is a good improvement over directly querying the catalog. The human-readable type format makes the test more maintainable and comprehensible.
89-94: Consistent type checking approachThis change follows the same pattern of using
function_types_eqwith explicit type names. This consistency across the codebase will make future maintenance easier and improve readability.tools/testers/dijkstra_pgtap_tests.sql (2)
187-194:Details
❓ Verification inconclusive
Ensure consistency in overload type checks for non-pgr_dijkstra functions
In this block, thefunction_types_eqcall is used to validate function signatures of overloaded functions. Please confirm that the apparent differences in array lengths (for instance, 11 parameters here versus 12 parameters in the pgr_dijkstra case) are deliberate and correctly reflect the underlying function definitions. A short comment outlining the intentional differences in expected argument counts could further enhance code clarity.
Action Required: Confirm and Document Signature Differences
The
function_types_eqcall in this block verifies the overload signatures for non-pgr_dijkstra functions. Notice that one overload uses an array with 12 type entries (as in the fourth definition) while the others use 10 or 11 parameters. Please verify that these differences are intentional and accurately reflect the underlying function definitions. If they are deliberate, consider adding a brief inline comment explaining the rationale behind the differing argument counts for clarity and future maintainability.
- File: tools/testers/dijkstra_pgtap_tests.sql (Lines 187–194)
161-168:Details
❓ Verification inconclusive
Streamline pgr_dijkstra type validation with
function_types_eq
This newly introduced block replaces the older OID-based checks with explicit text-array definitions, which improves clarity. Please verify that the order and number of types in each array precisely match the actual function signature forpgr_dijkstra. If it isn’t already clear from the context, adding a brief inline comment to describe the rationale for each array’s structure would help future maintainers.
🏁 Script executed:
#!/bin/bash # Step 1: Search for the pgr_dijkstra function definition across SQL files. echo "Searching for pgr_dijkstra function definition..." rg -n "CREATE FUNCTION.*pgr_dijkstra" . # Step 2: Display the test block snippet from tools/testers/dijkstra_pgtap_tests.sql for context. echo "Extracting lines 160 to 170 from tools/testers/dijkstra_pgtap_tests.sql..." sed -n '160,170p' tools/testers/dijkstra_pgtap_tests.sqlLength of output: 2810
Double-check
pgr_dijkstraType Arrays & Add Clarifying Inline Comments
- The new block in
tools/testers/dijkstra_pgtap_tests.sqlreplaces older OID-based checks with explicit text-array definitions. Please confirm that each of these five arrays exactly mirrors the corresponding function signature ofpgr_dijkstra(as defined, for example, insql/dijkstra/dijkstra.sql).- If the rationale behind each array’s structure isn’t immediately obvious from the context, please add a brief inline comment next to each array explaining which overload or parameter set it represents and why its ordering was chosen.
|
Update test is passing now: https://github.com/cvvergara/pgrouting/actions/runs/14201556147 |
2 participants
Fixes #2807
@pgRouting/admins
Summary by CodeRabbit