Quote username identifier appropriately #5613

nikkhils · 2023-04-25T14:58:51Z

Need to use quote_ident() on the user roles. Otherwise the extension scripts will fail.

mkindahl

Looks good. Can you add a test so that we do not repeat the mistake?

codecov · 2023-04-25T15:16:29Z

Codecov Report

Merging #5613 (9aa8d77) into main (2ce4bbc) will decrease coverage by 0.07%.
The diff coverage is n/a.

❗ Current head 9aa8d77 differs from pull request most recent head 2b0a0ac. Consider uploading reports for the commit 2b0a0ac to get more accurate results

@@            Coverage Diff             @@
##             main    #5613      +/-   ##
==========================================
- Coverage   90.92%   90.86%   -0.07%     
==========================================
  Files         229      229              
  Lines       54064    54059       -5     
==========================================
- Hits        49156    49119      -37     
- Misses       4908     4940      +32

see 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

mkindahl · 2023-04-25T17:52:32Z

Fixed changelog and added test case.

fabriziomello · 2023-04-25T18:11:37Z

sql/updates/2.10.1--2.10.2.sql

+    ALTER COLUMN owner TYPE regrole USING quote_ident(owner)::regrole,
+    ALTER COLUMN owner SET DEFAULT quote_ident(current_role)::regrole;


You can't change existing update/downgrade SQL scripts, you should create a new update/downgrade scripts in latest-dev.sql and reverse-dev.sql because users that already updated from 2.10.1 to 2.10.2 will not have your change.

This change should be part of the next update script, for example: 2.10.2--2.10.3.sql

You can't change existing update/downgrade SQL scripts, you should create a new update/downgrade scripts in latest-dev.sql and reverse-dev.sql because users that already updated from 2.10.1 to 2.10.2 will not have your change.

Not sure I follow the reasoning.

A user that already upgraded to 2.10.2 without error did not have a case-sensitive user name. If they had, they would fail to upgrade. In this case, the user name is the same before and after the upgrade.

A user that fails to upgrade to any version that includes this file (for example, 2.10.1 to 2.10.3) will fail because the user does not exist (I think we can assume that since it is not very likely that there multiple users with same user name but different case spelling). In this case we need to correct this file.

This change should be part of the next update script, for example: 2.10.2--2.10.3.sql

We need to correct the default in the next upgrade script as well, yes. If a case-sensitive user is added while on 2.10.2, they need to be able to update to the new version and not correcting the default would break this.

@fabriziomello only updated latest-dev and reverse-dev now

This is about consistent catalog definition across versions. If we modify update scripts that have already been packaged and distributed we end up with users having the same timescaledb version but different catalog definition which is something we should avoid.

svenklemm

Existing update scripts should not be changed as it makes the same version have different catalog definition based on which version of the script was run.

nikkhils · 2023-04-28T08:43:11Z

Existing update scripts should not be changed as it makes the same version have different catalog definition based on which version of the script was run.

Done. Updated only latest-dev and reverse-dev now.

svenklemm · 2023-04-28T09:04:33Z

Do we really need that much effort in latest-dev/reverse-dev? Can't we just alter the column default in place?

mkindahl

It would be good if you could check if it is sufficient to just change the default using ALTER. It seems like it is done in some previous upgrades when the default is changed, but it would be good if we do not need this extensive re-write.

Need to use quote_ident() on the user roles. Otherwise the extension scripts will fail. Co-authored-by: Mats Kindahl <mats@timescale.com>

nikkhils · 2023-04-28T10:08:29Z

Do we really need that much effort in latest-dev/reverse-dev? Can't we just alter the column default in place?

Well, given that we don't have any reports yet from the field about any related failures, I guess we could go with a default.

If the earlier upgrade without quoted_ident didn't fail for them then this will work. SO doing this for now

@kovetskiy

This release includes these noteworthy features: * compressed hypertable enhancements: * UPDATE/DELETE support * ON CONFLICT DO UPDATE * Join support for hierarchical Continougs Aggregates * performance improvements **Features** * timescale#5212 Allow pushdown of reference table joins * timescale#5221 Improve Realtime Continuous Aggregate performance * timescale#5252 Improve unique constraint support on compressed hypertables * timescale#5339 Support UPDATE/DELETE on compressed hypertables * timescale#5344 Enable JOINS for Hierarchical Continuous Aggregates * timescale#5361 Add parallel support for partialize_agg() * timescale#5417 Refactor and optimize distributed COPY * timescale#5454 Add support for ON CONFLICT DO UPDATE for compressed hypertables * timescale#5547 Skip Ordered Append when only 1 child node is present * timescale#5510 Propagate vacuum/analyze to compressed chunks * timescale#5584 Reduce decompression during constraint checking * timescale#5530 Optimize compressed chunk resorting **Bugfixes** * timescale#5396 Fix SEGMENTBY columns predicates to be pushed down * timescale#5427 Handle user-defined FDW options properly * timescale#5442 Decompression may have lost DEFAULT values * timescale#5459 Fix issue creating dimensional constraints * timescale#5570 Improve interpolate error message on datatype mismatch * timescale#5573 Fix unique constraint on compressed tables * timescale#5615 Add permission checks to run_job() * timescale#5614 Enable run_job() for telemetry job * timescale#5578 Fix on-insert decompression after schema changes * timescale#5613 Quote username identifier appropriately * timescale#5525 Fix tablespace for compressed hypertable and corresponding toast * timescale#5642 Fix ALTER TABLE SET with normal tables * timescale#5666 Reduce memory usage for distributed analyze * timescale#5668 Fix subtransaction resource owner **Thanks** * @kovetskiy and @DZDomi for reporting peformance regression in Realtime Continuous Aggregates * @ollz272 for reporting an issue with interpolate error messages

@kovetskiy

This release contains new features and bug fixes since the 2.10.3 release. We deem it moderate priority for upgrading. This release includes these noteworthy features: * Support for DML operations on compressed chunks: * UPDATE/DELETE support * Support for unique constraints on compressed chunks * Support for `ON CONFLICT DO UPDATE` * Support for `ON CONFLICT DO NOTHING` * Join support for hierarchical Continuous Aggregates **Features** * timescale#5212 Allow pushdown of reference table joins * timescale#5221 Improve Realtime Continuous Aggregate performance * timescale#5252 Improve unique constraint support on compressed hypertables * timescale#5339 Support UPDATE/DELETE on compressed hypertables * timescale#5344 Enable JOINS for Hierarchical Continuous Aggregates * timescale#5361 Add parallel support for partialize_agg() * timescale#5417 Refactor and optimize distributed COPY * timescale#5454 Add support for ON CONFLICT DO UPDATE for compressed hypertables * timescale#5547 Skip Ordered Append when only 1 child node is present * timescale#5510 Propagate vacuum/analyze to compressed chunks * timescale#5584 Reduce decompression during constraint checking * timescale#5530 Optimize compressed chunk resorting * timescale#5639 Support sending telemetry event reports **Bugfixes** * timescale#5396 Fix SEGMENTBY columns predicates to be pushed down * timescale#5427 Handle user-defined FDW options properly * timescale#5442 Decompression may have lost DEFAULT values * timescale#5459 Fix issue creating dimensional constraints * timescale#5570 Improve interpolate error message on datatype mismatch * timescale#5573 Fix unique constraint on compressed tables * timescale#5615 Add permission checks to run_job() * timescale#5614 Enable run_job() for telemetry job * timescale#5578 Fix on-insert decompression after schema changes * timescale#5613 Quote username identifier appropriately * timescale#5525 Fix tablespace for compressed hypertable and corresponding toast * timescale#5642 Fix ALTER TABLE SET with normal tables * timescale#5666 Reduce memory usage for distributed analyze * timescale#5668 Fix subtransaction resource owner **Thanks** * @kovetskiy and @DZDomi for reporting peformance regression in Realtime Continuous Aggregates * @ollz272 for reporting an issue with interpolate error messages

@kovetskiy

This release contains new features and bug fixes since the 2.10.3 release. We deem it moderate priority for upgrading. This release includes these noteworthy features: * Support for DML operations on compressed chunks: * UPDATE/DELETE support * Support for unique constraints on compressed chunks * Support for `ON CONFLICT DO UPDATE` * Support for `ON CONFLICT DO NOTHING` * Join support for hierarchical Continuous Aggregates **Features** * timescale#5212 Allow pushdown of reference table joins * timescale#5221 Improve Realtime Continuous Aggregate performance * timescale#5252 Improve unique constraint support on compressed hypertables * timescale#5339 Support UPDATE/DELETE on compressed hypertables * timescale#5344 Enable JOINS for Hierarchical Continuous Aggregates * timescale#5361 Add parallel support for partialize_agg() * timescale#5417 Refactor and optimize distributed COPY * timescale#5454 Add support for ON CONFLICT DO UPDATE for compressed hypertables * timescale#5547 Skip Ordered Append when only 1 child node is present * timescale#5510 Propagate vacuum/analyze to compressed chunks * timescale#5584 Reduce decompression during constraint checking * timescale#5530 Optimize compressed chunk resorting * timescale#5639 Support sending telemetry event reports **Bugfixes** * timescale#5396 Fix SEGMENTBY columns predicates to be pushed down * timescale#5427 Handle user-defined FDW options properly * timescale#5442 Decompression may have lost DEFAULT values * timescale#5459 Fix issue creating dimensional constraints * timescale#5570 Improve interpolate error message on datatype mismatch * timescale#5573 Fix unique constraint on compressed tables * timescale#5615 Add permission checks to run_job() * timescale#5614 Enable run_job() for telemetry job * timescale#5578 Fix on-insert decompression after schema changes * timescale#5613 Quote username identifier appropriately * timescale#5525 Fix tablespace for compressed hypertable and corresponding toast * timescale#5642 Fix ALTER TABLE SET with normal tables * timescale#5666 Reduce memory usage for distributed analyze * timescale#5668 Fix subtransaction resource owner **Thanks** * @kovetskiy and @DZDomi for reporting peformance regression in Realtime Continuous Aggregates * @ollz272 for reporting an issue with interpolate error messages

@kovetskiy

This release contains new features and bug fixes since the 2.10.3 release. We deem it moderate priority for upgrading. This release includes these noteworthy features: * Support for DML operations on compressed chunks: * UPDATE/DELETE support * Support for unique constraints on compressed chunks * Support for `ON CONFLICT DO UPDATE` * Support for `ON CONFLICT DO NOTHING` * Join support for hierarchical Continuous Aggregates **Features** * #5212 Allow pushdown of reference table joins * #5221 Improve Realtime Continuous Aggregate performance * #5252 Improve unique constraint support on compressed hypertables * #5339 Support UPDATE/DELETE on compressed hypertables * #5344 Enable JOINS for Hierarchical Continuous Aggregates * #5361 Add parallel support for partialize_agg() * #5417 Refactor and optimize distributed COPY * #5454 Add support for ON CONFLICT DO UPDATE for compressed hypertables * #5547 Skip Ordered Append when only 1 child node is present * #5510 Propagate vacuum/analyze to compressed chunks * #5584 Reduce decompression during constraint checking * #5530 Optimize compressed chunk resorting * #5639 Support sending telemetry event reports **Bugfixes** * #5396 Fix SEGMENTBY columns predicates to be pushed down * #5427 Handle user-defined FDW options properly * #5442 Decompression may have lost DEFAULT values * #5459 Fix issue creating dimensional constraints * #5570 Improve interpolate error message on datatype mismatch * #5573 Fix unique constraint on compressed tables * #5615 Add permission checks to run_job() * #5614 Enable run_job() for telemetry job * #5578 Fix on-insert decompression after schema changes * #5613 Quote username identifier appropriately * #5525 Fix tablespace for compressed hypertable and corresponding toast * #5642 Fix ALTER TABLE SET with normal tables * #5666 Reduce memory usage for distributed analyze * #5668 Fix subtransaction resource owner **Thanks** * @kovetskiy and @DZDomi for reporting peformance regression in Realtime Continuous Aggregates * @ollz272 for reporting an issue with interpolate error messages

@kovetskiy

This release contains new features and bug fixes since the 2.10.3 release. We deem it moderate priority for upgrading. This release includes these noteworthy features: * Support for DML operations on compressed chunks: * UPDATE/DELETE support * Support for unique constraints on compressed chunks * Support for `ON CONFLICT DO UPDATE` * Support for `ON CONFLICT DO NOTHING` * Join support for hierarchical Continuous Aggregates **Features** * timescale#5212 Allow pushdown of reference table joins * timescale#5221 Improve Realtime Continuous Aggregate performance * timescale#5252 Improve unique constraint support on compressed hypertables * timescale#5339 Support UPDATE/DELETE on compressed hypertables * timescale#5344 Enable JOINS for Hierarchical Continuous Aggregates * timescale#5361 Add parallel support for partialize_agg() * timescale#5417 Refactor and optimize distributed COPY * timescale#5454 Add support for ON CONFLICT DO UPDATE for compressed hypertables * timescale#5547 Skip Ordered Append when only 1 child node is present * timescale#5510 Propagate vacuum/analyze to compressed chunks * timescale#5584 Reduce decompression during constraint checking * timescale#5530 Optimize compressed chunk resorting * timescale#5639 Support sending telemetry event reports **Bugfixes** * timescale#5396 Fix SEGMENTBY columns predicates to be pushed down * timescale#5427 Handle user-defined FDW options properly * timescale#5442 Decompression may have lost DEFAULT values * timescale#5459 Fix issue creating dimensional constraints * timescale#5570 Improve interpolate error message on datatype mismatch * timescale#5573 Fix unique constraint on compressed tables * timescale#5615 Add permission checks to run_job() * timescale#5614 Enable run_job() for telemetry job * timescale#5578 Fix on-insert decompression after schema changes * timescale#5613 Quote username identifier appropriately * timescale#5525 Fix tablespace for compressed hypertable and corresponding toast * timescale#5642 Fix ALTER TABLE SET with normal tables * timescale#5666 Reduce memory usage for distributed analyze * timescale#5668 Fix subtransaction resource owner **Thanks** * @kovetskiy and @DZDomi for reporting peformance regression in Realtime Continuous Aggregates * @ollz272 for reporting an issue with interpolate error messages

@kovetskiy

This release contains new features and bug fixes since the 2.10.3 release. We deem it moderate priority for upgrading. This release includes these noteworthy features: * Support for DML operations on compressed chunks: * UPDATE/DELETE support * Support for unique constraints on compressed chunks * Support for `ON CONFLICT DO UPDATE` * Support for `ON CONFLICT DO NOTHING` * Join support for hierarchical Continuous Aggregates **Features** * timescale#5212 Allow pushdown of reference table joins * timescale#5221 Improve Realtime Continuous Aggregate performance * timescale#5252 Improve unique constraint support on compressed hypertables * timescale#5339 Support UPDATE/DELETE on compressed hypertables * timescale#5344 Enable JOINS for Hierarchical Continuous Aggregates * timescale#5361 Add parallel support for partialize_agg() * timescale#5417 Refactor and optimize distributed COPY * timescale#5454 Add support for ON CONFLICT DO UPDATE for compressed hypertables * timescale#5547 Skip Ordered Append when only 1 child node is present * timescale#5510 Propagate vacuum/analyze to compressed chunks * timescale#5584 Reduce decompression during constraint checking * timescale#5530 Optimize compressed chunk resorting * timescale#5639 Support sending telemetry event reports **Bugfixes** * timescale#5396 Fix SEGMENTBY columns predicates to be pushed down * timescale#5427 Handle user-defined FDW options properly * timescale#5442 Decompression may have lost DEFAULT values * timescale#5459 Fix issue creating dimensional constraints * timescale#5570 Improve interpolate error message on datatype mismatch * timescale#5573 Fix unique constraint on compressed tables * timescale#5615 Add permission checks to run_job() * timescale#5614 Enable run_job() for telemetry job * timescale#5578 Fix on-insert decompression after schema changes * timescale#5613 Quote username identifier appropriately * timescale#5525 Fix tablespace for compressed hypertable and corresponding toast * timescale#5642 Fix ALTER TABLE SET with normal tables * timescale#5666 Reduce memory usage for distributed analyze * timescale#5668 Fix subtransaction resource owner **Thanks** * @kovetskiy and @DZDomi for reporting peformance regression in Realtime Continuous Aggregates * @ollz272 for reporting an issue with interpolate error messages

nikkhils requested review from mkindahl and akuzm April 25, 2023 14:58

github-actions bot assigned nikkhils Apr 25, 2023

mkindahl reviewed Apr 25, 2023

View reviewed changes

mkindahl force-pushed the quote_ident branch from e2b8af1 to fcb30f8 Compare April 25, 2023 17:52

mkindahl self-assigned this Apr 25, 2023

mkindahl requested review from mkindahl and removed request for mkindahl April 25, 2023 18:02

fabriziomello reviewed Apr 25, 2023

View reviewed changes

svenklemm requested changes Apr 26, 2023

View reviewed changes

nikkhils force-pushed the quote_ident branch 2 times, most recently from 7a08771 to bd9df4c Compare April 28, 2023 08:42

nikkhils force-pushed the quote_ident branch 3 times, most recently from 8a6f017 to fd51969 Compare April 28, 2023 08:49

nikkhils requested a review from svenklemm April 28, 2023 08:51

nikkhils force-pushed the quote_ident branch from fd51969 to 6eb6f2a Compare April 28, 2023 09:09

mkindahl approved these changes Apr 28, 2023

View reviewed changes

svenklemm approved these changes Apr 28, 2023

View reviewed changes

Quote username identifier appropriately

2b0a0ac

Need to use quote_ident() on the user roles. Otherwise the extension scripts will fail. Co-authored-by: Mats Kindahl <mats@timescale.com>

nikkhils force-pushed the quote_ident branch from 6eb6f2a to 2b0a0ac Compare April 28, 2023 09:46

horzsolt added this to the TimescaleDB 2.11 milestone Apr 28, 2023

nikkhils merged commit ed8ca31 into timescale:main Apr 28, 2023
56 of 58 checks passed

nikkhils deleted the quote_ident branch April 28, 2023 11:23

kgyrtkirk mentioned this pull request May 17, 2023

Release 2.11.0 #5695

Merged

kgyrtkirk mentioned this pull request May 19, 2023

Release 2.11.0 - fix date #5702

Closed

mkindahl mentioned this pull request Aug 8, 2023

Add weird user names to update test #5930

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quote username identifier appropriately #5613

Quote username identifier appropriately #5613

nikkhils commented Apr 25, 2023

mkindahl left a comment

codecov bot commented Apr 25, 2023 •

edited

Loading

mkindahl commented Apr 25, 2023

fabriziomello Apr 25, 2023 •

edited

Loading

mkindahl Apr 26, 2023

nikkhils Apr 28, 2023

svenklemm Apr 28, 2023

svenklemm left a comment

nikkhils commented Apr 28, 2023

svenklemm commented Apr 28, 2023

mkindahl left a comment

nikkhils commented Apr 28, 2023

		ALTER COLUMN owner TYPE regrole USING quote_ident(owner)::regrole,
		ALTER COLUMN owner SET DEFAULT quote_ident(current_role)::regrole;

Quote username identifier appropriately #5613

Quote username identifier appropriately #5613

Conversation

nikkhils commented Apr 25, 2023

mkindahl left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 25, 2023 • edited Loading

Codecov Report

mkindahl commented Apr 25, 2023

fabriziomello Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

mkindahl Apr 26, 2023

Choose a reason for hiding this comment

nikkhils Apr 28, 2023

Choose a reason for hiding this comment

svenklemm Apr 28, 2023

Choose a reason for hiding this comment

svenklemm left a comment

Choose a reason for hiding this comment

nikkhils commented Apr 28, 2023

svenklemm commented Apr 28, 2023

mkindahl left a comment

Choose a reason for hiding this comment

nikkhils commented Apr 28, 2023

codecov bot commented Apr 25, 2023 •

edited

Loading

fabriziomello Apr 25, 2023 •

edited

Loading