-
Notifications
You must be signed in to change notification settings - Fork 959
Fixes for the new testgres 1.11.0 #7990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes for the new testgres 1.11.0 #7990
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7990 +/- ##
==========================================
+ Coverage 80.06% 82.13% +2.06%
==========================================
Files 190 250 +60
Lines 37181 46284 +9103
Branches 9450 11613 +2163
==========================================
+ Hits 29770 38014 +8244
- Misses 2997 3668 +671
- Partials 4414 4602 +188 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8714ce7
to
595d461
Compare
A new testgres framework was released that makes `PostgresNode::port` read-only. Fixed it by properly set the port when initializing a new node. References: https://github.com/postgrespro/testgres/releases/tag/1.11.0 postgrespro/testgres#234
595d461
to
e651aaf
Compare
|
||
-- Functions in schemas: | ||
-- * _timescaledb_internal | ||
-- * _timescaledb_functions | ||
-- * public | ||
SELECT n.nspname as "Schema", | ||
p.proname as "Name", | ||
pg_catalog.pg_get_function_result(p.oid) as "Result data type", | ||
pg_catalog.pg_get_function_arguments(p.oid) as "Argument data types", | ||
CASE p.prokind | ||
WHEN 'a' THEN 'agg' | ||
WHEN 'w' THEN 'window' | ||
WHEN 'p' THEN 'proc' | ||
ELSE 'func' | ||
END as "Type", | ||
CASE | ||
WHEN p.provolatile = 'i' THEN 'immutable' | ||
WHEN p.provolatile = 's' THEN 'stable' | ||
WHEN p.provolatile = 'v' THEN 'volatile' | ||
END as "Volatility", | ||
CASE | ||
WHEN p.proparallel = 'r' THEN 'restricted' | ||
WHEN p.proparallel = 's' THEN 'safe' | ||
WHEN p.proparallel = 'u' THEN 'unsafe' | ||
END as "Parallel", | ||
pg_catalog.pg_get_userbyid(p.proowner) as "Owner", | ||
CASE WHEN prosecdef THEN 'definer' ELSE 'invoker' END AS "Security", | ||
CASE WHEN pg_catalog.array_length(p.proacl, 1) = 0 THEN '(none)' ELSE pg_catalog.array_to_string(p.proacl, E'\n') END AS "Access privileges", | ||
l.lanname as "Language", | ||
p.prosrc as "Source code", | ||
CASE WHEN l.lanname IN ('internal', 'c') THEN p.prosrc END as "Internal name", | ||
pg_catalog.obj_description(p.oid, 'pg_proc') as "Description" | ||
FROM pg_catalog.pg_proc p | ||
LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace | ||
LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang | ||
WHERE n.nspname OPERATOR(pg_catalog.~) '^(_timescaledb_internal|_timescaledb_functions|public)$' COLLATE pg_catalog.default | ||
ORDER BY 1, 2, 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@melihmutlu added a custom SQL to get the functions output instead of relay on \df+
psql metacommand output that changes over Postgres versions.
-- Functions in schemas: | ||
-- * _timescaledb_internal | ||
-- * _timescaledb_functions | ||
-- * public | ||
SELECT n.nspname as "Schema", | ||
p.proname as "Name", | ||
pg_catalog.pg_get_function_result(p.oid) as "Result data type", | ||
pg_catalog.pg_get_function_arguments(p.oid) as "Argument data types", | ||
CASE p.prokind | ||
WHEN 'a' THEN 'agg' | ||
WHEN 'w' THEN 'window' | ||
WHEN 'p' THEN 'proc' | ||
ELSE 'func' | ||
END as "Type", | ||
CASE | ||
WHEN p.provolatile = 'i' THEN 'immutable' | ||
WHEN p.provolatile = 's' THEN 'stable' | ||
WHEN p.provolatile = 'v' THEN 'volatile' | ||
END as "Volatility", | ||
CASE | ||
WHEN p.proparallel = 'r' THEN 'restricted' | ||
WHEN p.proparallel = 's' THEN 'safe' | ||
WHEN p.proparallel = 'u' THEN 'unsafe' | ||
END as "Parallel", | ||
pg_catalog.pg_get_userbyid(p.proowner) as "Owner", | ||
CASE WHEN prosecdef THEN 'definer' ELSE 'invoker' END AS "Security", | ||
CASE WHEN pg_catalog.array_length(p.proacl, 1) = 0 THEN '(none)' ELSE pg_catalog.array_to_string(p.proacl, E'\n') END AS "Access privileges", | ||
l.lanname as "Language", | ||
p.prosrc as "Source code", | ||
CASE WHEN l.lanname IN ('internal', 'c') THEN p.prosrc END as "Internal name", | ||
pg_catalog.obj_description(p.oid, 'pg_proc') as "Description" | ||
FROM pg_catalog.pg_proc p | ||
LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace | ||
LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang | ||
WHERE n.nspname OPERATOR(pg_catalog.~) '^(_timescaledb_internal|_timescaledb_functions|public)$' COLLATE pg_catalog.default | ||
ORDER BY 1, 2, 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure a 30 line sql statement is an improvement over a 4 line psql meta commands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The \df+ output changed so I decided to hardcode this output on the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think \df is much easier to maintain, also this seems unrelated to the ci fix so we should probably separate this and get the ci fix in quickly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not unrelated because before we ran the psql from the newest version against the old cluster to produce exact the same output and don't have this change. Not PostgresNode::port
is read-only so we can't change the port on-the-fly to run psql from newest cluster against oldest. O also think that \df
is way easier to maintain but this post.catalog.sql
have \df+
since forever. Do u think it is not necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I think \df+ is useful to verify the result of update scripts, if we run the same psql version against both versions there should be no diff, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I think \df+ is useful to verify the result of update scripts, if we run the same psql version against both versions there should be no diff, right?
This is exactly what I tried to explain because now the PostgresNode::port
is read-only so I can't change it in order to run new psql version against old cluster. Check the .py changes.
A new testgres framework was released that makes
PostgresNode::port
read-only. Fixed it by properly set the port when initializing a new node.Failure due to the new testgres version: https://github.com/timescale/timescaledb/actions/runs/14589439220/job/40921093070?pr=7976
References:
https://github.com/postgrespro/testgres/releases/tag/1.11.0
postgrespro/testgres#234
Disable-check: force-changelog-file
Disable-check: approval-count