Re: Strange OSX make check-world failure

agora inbox for [email protected]  
help / color / mirror / Atom feed

Re: Strange OSX make check-world failure
11+ messages / 5 participants
[nested] [flat]

* Re: Strange OSX make check-world failure
@ 2018-12-07 05:18  Samuel Cochran <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Samuel Cochran @ 2018-12-07 05:18 UTC (permalink / raw)
  To: [email protected]

Hi folks 👋

Forgive me if I'm getting the mailing list etiquette wrong — first time poster.

I ended up sitting next to Thomas Munro at PGDU 2018 and talking about testing. While trying to get `make check` running on my macbook, I think I may have fixed this issue.

System Integrity Protection strips dynamic linker (dyld) environment variables, such as DYLD_LIBRARY_PATH, during exec(2) [1] so we need to rewrite the load paths inside binaries when relocating then during make temp-install before make check on darwin. Homebrew does something similar [2]. I've attached a patch which adjust the Makefile and gets make check working on my machine with SIP in tact.

Cheers,
Sam

  [1]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Prote...
  [2]: https://github.com/Homebrew/brew/blob/77e6a927504c51a1393a0a6ccaf6f2611ac4a9d5/Library/Homebrew/os/m...


On Tue, Sep 18, 2018, at 8:39 AM, Tom Lane wrote:
> Thomas Munro <[email protected]> writes:
> > On Tue, Sep 18, 2018 at 2:14 AM Tom Lane <[email protected]> wrote:
> >> "make check" generally won't work on OSX unless you've disabled SIP:
> >> https://www.howtogeek.com/230424/how-to-disable-system-integrity-protection-on-a-mac-and-why-you-sho...
> 
> > Aha!  It looks like it was important to run "make install" before
> > running those tests.
> 
> Right.  If you don't want to disable SIP, you can work around it by always
> doing "make install" before "make check".  Kind of a PITA though.
> 
> 			regards, tom lane
> 
> 

From cdfe53a93453d8cdf12cfaaea13574365fbba66b Mon Sep 17 00:00:00 2001
From: Samuel Cochran <[email protected]>
Date: Fri, 7 Dec 2018 15:27:30 +1100
Subject: [PATCH] Fix `make check` on darwin

System Integrity Protection strips dynamic linker (dyld) environment
variables, such as DYLD_LIBRARY_PATH, during exec(2), so we need to
rewrite the load paths inside binaries when relocating then during make
temp-install before make check on darwin.

https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Prote...

diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 956fd274cd..48f2e2bcc7 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -390,6 +390,10 @@ ifeq ($(MAKELEVEL),0)
 	rm -rf '$(abs_top_builddir)'/tmp_install
 	$(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log
 	$(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
+	# darwin doesn't propagate DYLD_* vars due to system integrity
+	# protection, so we need to rewrite the load commands inside the
+	# binaries when relocating them
+	$(if $(filter $(PORTNAME),darwin),find '$(abs_top_builddir)/tmp_install$(bindir)' -type f -exec install_name_tool -change '$(libdir)/libpq.5.dylib' '$(abs_top_builddir)/tmp_install$(libdir)/libpq.5.dylib' {} \;)
 endif
 	$(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install >>'$(abs_top_builddir)'/tmp_install/log/install.log || exit; done)
 endif


Attachments:

  [text/plain] installname.patch (1.6K, ../../1544159936.229421.1601719680.36DF628A@webmail.messagingengine.com/2-installname.patch)
  download | inline diff:
From cdfe53a93453d8cdf12cfaaea13574365fbba66b Mon Sep 17 00:00:00 2001
From: Samuel Cochran <[email protected]>
Date: Fri, 7 Dec 2018 15:27:30 +1100
Subject: [PATCH] Fix `make check` on darwin

System Integrity Protection strips dynamic linker (dyld) environment
variables, such as DYLD_LIBRARY_PATH, during exec(2), so we need to
rewrite the load paths inside binaries when relocating then during make
temp-install before make check on darwin.

https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html

diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 956fd274cd..48f2e2bcc7 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -390,6 +390,10 @@ ifeq ($(MAKELEVEL),0)
 	rm -rf '$(abs_top_builddir)'/tmp_install
 	$(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log
 	$(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
+	# darwin doesn't propagate DYLD_* vars due to system integrity
+	# protection, so we need to rewrite the load commands inside the
+	# binaries when relocating them
+	$(if $(filter $(PORTNAME),darwin),find '$(abs_top_builddir)/tmp_install$(bindir)' -type f -exec install_name_tool -change '$(libdir)/libpq.5.dylib' '$(abs_top_builddir)/tmp_install$(libdir)/libpq.5.dylib' {} \;)
 endif
 	$(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install >>'$(abs_top_builddir)'/tmp_install/log/install.log || exit; done)
 endif


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: Strange OSX make check-world failure
@ 2018-12-07 06:26  Tom Lane <[email protected]>
  parent: Samuel Cochran <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Tom Lane @ 2018-12-07 06:26 UTC (permalink / raw)
  To: Samuel Cochran <[email protected]>; +Cc: [email protected]

Samuel Cochran <[email protected]> writes:
> System Integrity Protection strips dynamic linker (dyld) environment variables, such as DYLD_LIBRARY_PATH, during exec(2) [1]

Yeah.  I wish Apple would just fix that silliness ... I'll spare you the
rant about why it's stupid, but it is.  (BTW, last I looked, it's not
exec(2) per se that's doing the damage; the problem is that we're
invoking a sub-shell that's considered a protected program for some
reason, and it's only the use of that that causes DYLD_LIBRARY_PATH
to get removed from the process environment.)

> so we need to rewrite the load paths inside binaries when relocating then during make temp-install before make check on darwin.

Interesting proposal, but I think it needs work.

* As coded, this only fixes the problem for references to libpq, not
any of our other shared libraries.

* It's also unpleasant that it hard-wires knowledge of libpq's version
numbering in a place pretty far removed from anywhere that should know
that.

* Just to be annoying, this won't work at all on 32-bit OSX versions
unless we link everything with -headerpad_max_install_names.  (I know
Apple forgot about 32-bit machines long ago, but our buildfarm hasn't.)

* Speaking of not working, I don't think this "find" invocation will
report any failure exits from install_name_tool.

* This doesn't fix anything for executables that never get installed,
for instance isolationtester.

We could probably fix the first four problems with some more sweat,
but I'm not seeing a plausible answer to the last one.  Overwriting
isolationtester's rpath to make "make check" work would just break
it for "make installcheck".

			regards, tom lane

^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: Strange OSX make check-world failure
@ 2018-12-08 03:20  Samuel Cochran <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Samuel Cochran @ 2018-12-08 03:20 UTC (permalink / raw)
  To: [email protected]

On Fri, Dec 7, 2018, at 5:26 PM, Tom Lane wrote:
> Interesting proposal, but I think it needs work.

Absolutely! I only hacked it together to the point that it worked on my laptop and illustrated the approach. :-)

> * As coded, this only fixes the problem for references to libpq, not
> any of our other shared libraries.

None of the the other shared libraries are referenced by the modified binaries:

$ for bin in tmp_install/usr/local/pgsql/bin/*; do otool -L $bin; done | grep dylib | sort -u
	.../tmp_install/usr/local/pgsql/lib/libpq.5.dylib (compatibility version 5.0.0, current version 5.12.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.200.5)
	/usr/lib/libedit.3.dylib (compatibility version 2.0.0, current version 3.0.0)
	/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)

But I agree it would be nice to make it work in potential future cases, too.

> * It's also unpleasant that it hard-wires knowledge of libpq's version
> numbering in a place pretty far removed from anywhere that should know
> that.

Ideally it would iterate the binaries, iterate the load commands, and rewrite each.

> * Just to be annoying, this won't work at all on 32-bit OSX versions
> unless we link everything with -headerpad_max_install_names.  (I know
> Apple forgot about 32-bit machines long ago, but our buildfarm hasn't.)

We can make the references relative which would dramatically decrease the sizes.

> * Speaking of not working, I don't think this "find" invocation will
> report any failure exits from install_name_tool.

If we iterate more carefully, as above, then failures should be reported and cause an abort.

> * This doesn't fix anything for executables that never get installed,
> for instance isolationtester.
> 
> We could probably fix the first four problems with some more sweat,
> but I'm not seeing a plausible answer to the last one.  Overwriting
> isolationtester's rpath to make "make check" work would just break
> it for "make installcheck".

Ah, sorry, I'm not super familiar yet with the build process so missed this bit. But I think executable-relative paths will fix.

I tried using this line instead and `make check` and `make installcheck` both work for me. It's awful, I'm not super fluent in Makefile so I'm sure it could be 100X better, and probably isn't quoted correctly, but the approach itself works. I couldn't quickly figure out a portable way to generate a relative path from bindir to libdir which would be a great improvement.

$(if $(filter $(PORTNAME),darwin),for binary in $(abs_top_builddir)/tmp_install$(bindir)/*; do for dylib in $$(otool -L $$binary | tail +2 | awk '{ print $$1 }' | grep '$(libdir)'); do install_name_tool -change $$dylib @executable_path/../lib/$${dylib##*/} $$binary || exit $$?; done; done)

Cheers,
Sam

^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* [PATCH 5/7] fix
@ 2020-03-19 01:54  Tomas Vondra <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Tomas Vondra @ 2020-03-19 01:54 UTC (permalink / raw)

---
 .../postgres_fdw/expected/postgres_fdw.out    |  4 ++--
 src/backend/executor/execExpr.c               |  5 +++--
 src/backend/executor/nodeAgg.c                | 20 +++++++++----------
 src/backend/optimizer/util/pathnode.c         |  4 ++--
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 62c2697920..fc0ed2f4d5 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -3448,8 +3448,8 @@ select c2, sum(c1) from ft1 where c2 < 3 group by rollup(c2) order by 1 nulls la
    Sort Key: ft1.c2
    ->  MixedAggregate
          Output: c2, sum(c1)
-         Hash Key: ft1.c2
          Group Key: ()
+         Hash Key: ft1.c2
          ->  Foreign Scan on public.ft1
                Output: c2, c1
                Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE ((c2 < 3))
@@ -3473,8 +3473,8 @@ select c2, sum(c1) from ft1 where c2 < 3 group by cube(c2) order by 1 nulls last
    Sort Key: ft1.c2
    ->  MixedAggregate
          Output: c2, sum(c1)
-         Hash Key: ft1.c2
          Group Key: ()
+         Hash Key: ft1.c2
          ->  Foreign Scan on public.ft1
                Output: c2, c1
                Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE ((c2 < 3))
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 07789501f7..669843faf5 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2937,7 +2937,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase, bool nullcheck)
 	PlanState  *parent = &aggstate->ss.ps;
 	ExprEvalStep scratch = {0};
 	bool		isCombine = DO_AGGSPLIT_COMBINE(aggstate->aggsplit);
-	ListCell	*lc;
+	ListCell   *lc;
 	LastAttnumInfo deform = {0, 0, 0};
 
 	state->expr = (Expr *) aggstate;
@@ -2978,6 +2978,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase, bool nullcheck)
 		NullableDatum *strictargs = NULL;
 		bool	   *strictnulls = NULL;
 		int			argno;
+		int			setno;
 		ListCell   *bail;
 
 		/*
@@ -3155,7 +3156,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase, bool nullcheck)
 		 * grouping set). Do so for both sort and hash based computations, as
 		 * applicable.
 		 */
-		for (int setno = 0; setno < phase->numsets; setno++)
+		for (setno = 0; setno < phase->numsets; setno++)
 		{
 			ExecBuildAggTransCall(state, aggstate, &scratch, trans_fcinfo,
 								  pertrans, transno, setno, phase, nullcheck);
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 20c5eb98b3..38d0bd5895 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -333,7 +333,7 @@ initialize_phase(AggState *aggstate, int newphase)
 	AggStatePerPhaseSort persort;
 
 	Assert(newphase == 0 || newphase == aggstate->current_phase + 1);
-	
+
 	/* Don't use aggstate->phase here, it might not be initialized yet*/
 	current_phase = aggstate->phases[aggstate->current_phase];
 
@@ -1516,7 +1516,7 @@ hash_agg_entry_size(int numAggs, Size tupleWidth, Size transitionSpace)
  * When called, CurrentMemoryContext should be the per-query context. The
  * already-calculated hash value for the tuple must be specified.
  */
-static void 
+static void
 lookup_hash_entry(AggState *aggstate, AggStatePerPhaseHash perhash, uint32 hash)
 {
 	TupleTableSlot *hashslot = perhash->hashslot;
@@ -1724,7 +1724,7 @@ agg_retrieve_direct(AggState *aggstate)
 					numGroupingSets = aggstate->phase->numsets;
 					node = aggstate->phase->aggnode;
 					numReset = numGroupingSets;
-					pergroups = aggstate->phase->pergroups; 
+					pergroups = aggstate->phase->pergroups;
 				}
 				else
 				{
@@ -2123,7 +2123,7 @@ agg_retrieve_hash_table(AggState *aggstate)
 				 */
 				select_current_set(aggstate, 0, true);
 				initialize_phase(aggstate, aggstate->current_phase + 1);
-				perhash = (AggStatePerPhaseHash) aggstate->phase;	
+				perhash = (AggStatePerPhaseHash) aggstate->phase;
 				ResetTupleHashIterator(perhash->hashtable, &perhash->hashiter);
 
 				continue;
@@ -2269,7 +2269,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 
 	aggstate->maxsets = numGroupingSets;
 	aggstate->numphases = 1 + list_length(node->chain);
-	
+
 	/*
 	 * The first phase is not sorted, agg need to do its own sort. See
 	 * agg_sort_input(), this can only happen in groupingsets case.
@@ -2390,7 +2390,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 	numaggs = aggstate->numaggs;
 	Assert(numaggs == list_length(aggstate->aggs));
 
-	/* 
+	/*
 	 * For each phase, prepare grouping set data and fmgr lookup data for
 	 * compare functions.  Accumulate all_grouped_cols in passing.
 	 */
@@ -2430,7 +2430,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 
 			all_grouped_cols = bms_add_members(all_grouped_cols, cols);
 
-			/* 
+			/*
 			 * Initialize pergroup state. For AGG_HASHED, all groups do transition
 			 * on the fly, all pergroup states are kept in hashtable, everytime
 			 * a tuple is processed, lookup_hash_entry() choose one group and
@@ -2499,7 +2499,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 				phasedata->grouped_cols = NULL;
 			}
 
-			/* 
+			/*
 			 * Initialize pergroup states for AGG_SORTED/AGG_PLAIN/AGG_MIXED
 			 * phases, each set only have one group on the fly, all groups in
 			 * a set can reuse a pergroup state. Unlike AGG_HASHED, we
@@ -3610,8 +3610,8 @@ ExecReScanAgg(AggState *node)
 					   sizeof(AggStatePerGroupData) * node->numaggs);
 		}
 
-		/* 
-		 * the agg did its own first sort using tuplesort and the first
+		/*
+		 * The agg did its own first sort using tuplesort and the first
 		 * tuplesort is kept (see initialize_phase), if the subplan does
 		 * not have any parameter changes, and none of our own parameter
 		 * changes affect input expressions of the aggregated functions,
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 2dfa3fa17e..ff8f676dfb 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -2983,7 +2983,7 @@ create_agg_path(PlannerInfo *root,
  * 'rollups' is a list of RollupData nodes
  * 'agg_costs' contains cost info about the aggregate functions to be computed
  * 'numGroups' is the estimated total number of groups
- * 'is_sorted' is the input sorted in the group cols of first rollup 
+ * 'is_sorted' is the input sorted in the group cols of first rollup
  */
 GroupingSetsPath *
 create_groupingsets_path(PlannerInfo *root,
@@ -3098,7 +3098,7 @@ create_groupingsets_path(PlannerInfo *root,
 			AggStrategy	rollup_strategy;
 			Path	sort_path;	/* dummy for result of cost_sort */
 			Path	agg_path;	/* dummy for result of cost_agg */
-			
+
 			sort_path.startup_cost = 0;
 			sort_path.total_cost = 0;
 			sort_path.rows = subpath->rows;
-- 
2.21.1


--4ms6w442s2ji2wqe
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: attachment;
	filename="0006-Parallel-grouping-sets.patch"
Content-Transfer-Encoding: 8bit



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* [PATCH 7/7] fix
@ 2020-03-19 02:02  Tomas Vondra <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Tomas Vondra @ 2020-03-19 02:02 UTC (permalink / raw)

---
 src/backend/executor/nodeAgg.c       | 20 ++++++++++----------
 src/backend/jit/llvm/llvmjit_expr.c  |  2 +-
 src/backend/optimizer/plan/planner.c | 12 ++++++------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index f7b98dd798..51c7f229e2 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -360,7 +360,7 @@ initialize_phase(AggState *aggstate, int newphase)
 		if (persort->store_in)
 		{
 			tuplestore_end(persort->store_in);
-			persort->store_in = NULL;	
+			persort->store_in = NULL;
 		}
 	}
 
@@ -2017,7 +2017,7 @@ agg_preprocess_groupingsets(AggState *aggstate)
 	/* Initialize tuples storage for each aggregate phases */
 	for (phaseidx = 0; phaseidx < aggstate->numphases; phaseidx++)
 	{
-		phase = aggstate->phases[phaseidx];	
+		phase = aggstate->phases[phaseidx];
 
 		if (!phase->is_hashed)
 		{
@@ -2039,12 +2039,12 @@ agg_preprocess_groupingsets(AggState *aggstate)
 			}
 			else
 			{
-				persort->store_in = tuplestore_begin_heap(false, false, work_mem);	
+				persort->store_in = tuplestore_begin_heap(false, false, work_mem);
 			}
 		}
 		else
 		{
-			/* 
+			/*
 			 * If it's a AGG_HASHED, we don't need a storage to store
 			 * the tuples for later process, we can do the transition
 			 * immediately.
@@ -2422,7 +2422,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 	aggstate->maxsets = numGroupingSets;
 	aggstate->numphases = 1 + list_length(node->chain);
 
-	/* 
+	/*
 	 * We are doing final stage of partial groupingsets, do preprocess
 	 * to input tuples first, redirect the tuples to according aggregate
 	 * phases. See agg_preprocess_groupingsets().
@@ -2431,7 +2431,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 	{
 		aggstate->groupingsets_preprocess = true;
 
-		/* 
+		/*
 		 * Allocate gsetid <-> phases mapping, in final stage of
 		 * partial groupingsets, all grouping sets are extracted
 		 * to individual phases, so the number of sets is equal
@@ -2449,7 +2449,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 	 * agg_sort_input(), this can only happen in groupingsets case.
 	 */
 	if (node->sortnode)
-		aggstate->input_sorted = false;	
+		aggstate->input_sorted = false;
 
 	aggstate->aggcontexts = (ExprContext **)
 		palloc0(sizeof(ExprContext *) * numGroupingSets);
@@ -2626,7 +2626,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 				phasedata->setno_gsetids[0] = gs->setId;
 			}
 
-			/* 
+			/*
 			 * Initialize pergroup state. For AGG_HASHED, all groups do transition
 			 * on the fly, all pergroup states are kept in hashtable, everytime
 			 * a tuple is processed, lookup_hash_entry() choose one group and
@@ -2688,7 +2688,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
 					phasedata->grouped_cols[i] = cols;
 					phasedata->gset_lengths[i] = current_length;
 
-					/* 
+					/*
 					 * In the initial stage of partial grouping sets, it provides extra
 					 * grouping sets ID in the targetlist, fill the setno <-> gsetid
 					 * map, so EEOP_GROUPING_SET_ID can evaluate correct gsetid for
@@ -3871,7 +3871,7 @@ ExecReScanAgg(AggState *node)
 			}
 		}
 
-		/* 
+		/*
 		 * if the agg is doing final stage of partial groupingsets, reset the
 		 * flag to do groupingsets preprocess again.
 		 */
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index f442442269..f70eaabd0c 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -1893,7 +1893,7 @@ llvm_compile_expr(ExprState *state)
 					v_aggstatep =
 						LLVMBuildBitCast(b, v_parent, l_ptr(StructAggState), "");
 
-					/* 
+					/*
 					 * op->resvalue =
 					 * aggstate->phase->setno_gsetids
 					 * [aggstate->current_set]
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index cedd3e1c9d..a0186091a1 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4211,7 +4211,7 @@ create_ordinary_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
  * added to grouped_rel->pathlist. And aggsplit value is not sufficient to
  * make a decision.
  */
-static void 
+static void
 consider_groupingsets_paths(PlannerInfo *root,
 							RelOptInfo *grouped_rel,
 							Path *path,
@@ -4601,7 +4601,7 @@ consider_groupingsets_paths(PlannerInfo *root,
 	}
 }
 
-/* 
+/*
  * If we are combining the partial groupingsets aggregation, the input is
  * mixed with tuples from different grouping sets, executor dispatch the
  * tuples to different rollups (phases) according to the grouping set id.
@@ -4644,7 +4644,7 @@ extract_final_rollups(PlannerInfo *root,
 			{
 				new_rollup->groupClause = NIL;
 				new_rollup->gsets_data = list_make1(gs);
-				new_rollup->gsets = list_make1(NIL); 
+				new_rollup->gsets = list_make1(NIL);
 				new_rollup->hashable = false;
 				new_rollup->is_hashed = false;
 			}
@@ -5364,7 +5364,7 @@ make_partial_grouping_target(PlannerInfo *root,
 
 	add_new_columns_to_pathtarget(partial_target, non_group_exprs);
 
-	/* 
+	/*
 	 * We are generate partial groupingsets path, add an expression to show
 	 * the grouping set ID for a tuple, so in the final stage, executor can
 	 * know which set this tuple belongs to and combine them.
@@ -6985,7 +6985,7 @@ create_partial_grouping_paths(PlannerInfo *root,
 													 path,
 													 root->group_pathkeys,
 													 -1.0);
-				
+
 				if (parse->hasAggs)
 					add_partial_path(partially_grouped_rel, (Path *)
 									 create_agg_path(root,
@@ -7059,7 +7059,7 @@ create_partial_grouping_paths(PlannerInfo *root,
 										AGGSPLIT_INITIAL_SERIAL,
 										add_partial_path);
 		}
-		else 
+		else
 		{
 			hashaggtablesize =
 				estimate_hashagg_tablesize(cheapest_partial_path,
-- 
2.21.1


--4ms6w442s2ji2wqe--





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* [PATCH 4/8] fix
@ 2020-03-19 15:55  Tomas Vondra <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Tomas Vondra @ 2020-03-19 15:55 UTC (permalink / raw)

---
 src/backend/commands/explain.c       |  6 ++--
 src/backend/executor/execProcnode.c  | 11 ++++----
 src/backend/optimizer/plan/planner.c | 41 +++++++++++++++++-----------
 src/backend/utils/sort/tuplesort.c   | 10 +++++--
 src/include/nodes/plannodes.h        |  1 -
 5 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index dd4600a214..0256dd42f1 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -2767,7 +2767,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 
 		foreach(methodCell, groupInfo->sortMethods)
 		{
-			const	   *sortMethodName = tuplesort_method_name(methodCell->int_value);
+			const char *sortMethodName = tuplesort_method_name(methodCell->int_value);
 
 			methodNames = lappend(methodNames, sortMethodName);
 		}
@@ -2776,7 +2776,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 		if (groupInfo->maxMemorySpaceUsed > 0)
 		{
 			long		avgSpace = groupInfo->totalMemorySpaceUsed / groupInfo->groupCount;
-			const	   *spaceTypeName;
+			const char *spaceTypeName;
 
 			ExplainPropertyInteger("Average Sort Space Used", "kB", avgSpace, es);
 			ExplainPropertyInteger("Maximum Sort Space Used", "kB",
@@ -2787,7 +2787,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 		if (groupInfo->maxDiskSpaceUsed > 0)
 		{
 			long		avgSpace = groupInfo->totalDiskSpaceUsed / groupInfo->groupCount;
-			const	   *spaceTypeName;
+			const char *spaceTypeName;
 
 			ExplainPropertyInteger("Average Sort Space Used", "kB", avgSpace, es);
 			ExplainPropertyInteger("Maximum Sort Space Used", "kB",
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index d15a86a706..5662e7d742 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -852,12 +852,13 @@ ExecSetTupleBound(int64 tuples_needed, PlanState *child_node)
 	else if (IsA(child_node, IncrementalSortState))
 	{
 		/*
-		 * If it is a Sort node, notify it that it can use bounded sort.
+		 * If it is an IncrementalSort node, notify it that it can use bounded
+		 * sort.
 		 *
-		 * Note: it is the responsibility of nodeSort.c to react properly to
-		 * changes of these parameters.  If we ever redesign this, it'd be a
-		 * good idea to integrate this signaling with the parameter-change
-		 * mechanism.
+		 * Note: it is the responsibility of nodeIncrementalSort.c to react
+		 * properly to changes of these parameters.  If we ever redesign this,
+		 * it'd be a good idea to integrate this signaling with the
+		 * parameter-change mechanism.
 		 */
 		IncrementalSortState *sortState = (IncrementalSortState *) child_node;
 
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index b2f4aaadb5..0e01cf8cb1 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4869,7 +4869,7 @@ create_distinct_paths(PlannerInfo *root,
 	else
 	{
 		Size		hashentrysize = hash_agg_entry_size(
-														0, cheapest_input_path->pathtarget->width, 0);
+			0, cheapest_input_path->pathtarget->width, 0);
 
 		/* Allow hashing only if hashtable is predicted to fit in work_mem */
 		allow_hash = (hashentrysize * numDistinctRows <= work_mem * 1024L);
@@ -4932,6 +4932,9 @@ create_distinct_paths(PlannerInfo *root,
  * target: the output tlist the result Paths must emit
  * limit_tuples: estimated bound on the number of output tuples,
  *		or -1 if no LIMIT or couldn't estimate
+ *
+ * XXX This only looks at sort_pathkeys. I wonder if it needs to look at the
+ * other pathkeys (grouping, ...) like generate_useful_gather_paths.
  */
 static RelOptInfo *
 create_ordered_paths(PlannerInfo *root,
@@ -5002,23 +5005,29 @@ create_ordered_paths(PlannerInfo *root,
 
 				add_path(ordered_rel, sorted_path);
 			}
-			if (enable_incrementalsort && presorted_keys > 0)
-			{
-				/* Also consider incremental sort. */
-				sorted_path = (Path *) create_incremental_sort_path(root,
-																	ordered_rel,
-																	input_path,
-																	root->sort_pathkeys,
-																	presorted_keys,
-																	limit_tuples);
 
-				/* Add projection step if needed */
-				if (sorted_path->pathtarget != target)
-					sorted_path = apply_projection_to_path(root, ordered_rel,
-														   sorted_path, target);
+			/* With incremental sort disabled, don't build those paths. */
+			if (!enable_incrementalsort)
+				continue;
 
-				add_path(ordered_rel, sorted_path);
-			}
+			/* Likewise, if the path can't be used for incremental sort. */
+			if (!presorted_keys)
+				continue;
+
+			/* Also consider incremental sort. */
+			sorted_path = (Path *) create_incremental_sort_path(root,
+																ordered_rel,
+																input_path,
+																root->sort_pathkeys,
+																presorted_keys,
+																limit_tuples);
+
+			/* Add projection step if needed */
+			if (sorted_path->pathtarget != target)
+				sorted_path = apply_projection_to_path(root, ordered_rel,
+													   sorted_path, target);
+
+			add_path(ordered_rel, sorted_path);
 		}
 	}
 
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 583551d197..77c15ebd78 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -256,8 +256,8 @@ struct Tuplesortstate
 	bool		isMaxSpaceDisk; /* true when maxSpace is value for on-disk
 								 * space, false when it's value for in-memory
 								 * space */
-	TupSortStatus maxSpaceStatus;	/* sort status when maxSpace was reached */
-	MemoryContext maincontext;	/* memory context for tuple sort metadata that
+	TupSortStatus	maxSpaceStatus;	/* sort status when maxSpace was reached */
+	MemoryContext	maincontext;	/* memory context for tuple sort metadata that
 								 * persists across multiple batches */
 	MemoryContext sortcontext;	/* memory context holding most sort data */
 	MemoryContext tuplecontext; /* sub-context of sortcontext for tuple data */
@@ -803,6 +803,9 @@ tuplesort_begin_common(int workMem, SortCoordinate coordinate,
 	return state;
 }
 
+/*
+ * XXX Missing comment.
+ */
 static void
 tuplesort_begin_batch(Tuplesortstate *state)
 {
@@ -1288,6 +1291,9 @@ tuplesort_set_bound(Tuplesortstate *state, int64 bound)
 	state->sortKeys->abbrev_full_comparator = NULL;
 }
 
+/*
+ * XXX Missing comment.
+ */
 bool
 tuplesort_used_bound(Tuplesortstate *state)
 {
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index fe4046b64b..136d794219 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -774,7 +774,6 @@ typedef struct Sort
 	bool	   *nullsFirst;		/* NULLS FIRST/LAST directions */
 } Sort;
 
-
 /* ----------------
  *		incremental sort node
  * ----------------
-- 
2.21.1


--dfcjsgdukgytabqd
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename*0=v39-0005-Consider-incremental-sort-paths-in-additional-places;
	filename*1=".patch"



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* [PATCH 6/8] fix
@ 2020-03-19 17:26  Tomas Vondra <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Tomas Vondra @ 2020-03-19 17:26 UTC (permalink / raw)

---
 src/backend/optimizer/path/allpaths.c | 62 +++++++++++++++++----------
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 9a92948fe3..6838a238cd 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2764,6 +2764,14 @@ find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel)
  * order matches the final output ordering for the overall query we're
  * planning, or because it enables an efficient merge join.  Here, we try
  * to figure out which pathkeys to consider.
+ *
+ * This allows us to do incremental sort on top of an index scan under a gather
+ * merge node, i.e. parallelized.
+ *
+ * XXX At the moment this can only ever return a list with a single element,
+ * because it looks at query_pathkeys only. So we might return the pathkeys
+ * directly, but it seems plausible we'll want to consider other orderings
+ * in the future.
  */
 static List *
 get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
@@ -2772,8 +2780,8 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
 	ListCell   *lc;
 
 	/*
-	 * Pushing the query_pathkeys to the remote server is always worth
-	 * considering, because it might let us avoid a local sort.
+	 * Considering query_pathkeys is always worth it, because it might let us
+	 * avoid a local sort.
 	 */
 	if (root->query_pathkeys)
 	{
@@ -2786,14 +2794,9 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
 			Expr	   *em_expr;
 
 			/*
-			 * The planner and executor don't have any clever strategy for
-			 * taking data sorted by a prefix of the query's pathkeys and
-			 * getting it to be sorted by all of those pathkeys. We'll just
-			 * end up re-sorting the entire data set.  So, unless we can push
-			 * down all of the query pathkeys, forget it.
-			 *
-			 * is_foreign_expr would detect volatile expressions as well, but
-			 * checking ec_has_volatile here saves some cycles.
+			 * We can't use incremental sort for pathkeys containing volatile
+			 * expressions. We could walk the exppression itself, but checking
+			 * ec_has_volatile here saves some cycles.
 			 */
 			if (pathkey_ec->ec_has_volatile ||
 				!(em_expr = find_em_expr_for_rel(pathkey_ec, rel)))
@@ -2803,10 +2806,6 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
 			}
 		}
 
-		/*
-		 * This ends up allowing us to do incremental sort on top of an index
-		 * scan all parallelized under a gather merge node.
-		 */
 		if (query_pathkeys_ok)
 			useful_pathkeys_list = list_make1(list_copy(root->query_pathkeys));
 	}
@@ -2819,10 +2818,10 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
  *		Generate parallel access paths for a relation by pushing a Gather or
  *		Gather Merge on top of a partial path.
  *
- * Unlike generate_gather_paths, this does not look only at pathkeys of the
- * input paths (aiming to preserve the ordering). It also considers ordering
- * that might be useful by nodes above the gather merge node, and tries to
- * add a sort (regular or incremental) to provide that.
+ * Unlike plain generate_gather_paths, this looks both at pathkeys of input
+ * paths (aiming to preserve the ordering), but also considers ordering that
+ * might be useful for nodes above the gather merge node, and tries to add
+ * a sort (regular or incremental) to provide that.
  */
 void
 generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)
@@ -2841,7 +2840,7 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
 	if (override_rows)
 		rowsp = &rows;
 
-	/* generate the regular gather merge paths */
+	/* generate the regular gather (merge) paths */
 	generate_gather_paths(root, rel, override_rows);
 
 	/* when incremental sort is disabled, we're done */
@@ -2851,7 +2850,7 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
 	/* consider incremental sort for interesting orderings */
 	useful_pathkeys_list = get_useful_pathkeys_for_relation(root, rel);
 
-	/* used for explicit sort paths */
+	/* used for explicit (full) sort paths */
 	cheapest_partial_path = linitial(rel->partial_pathlist);
 
 	/*
@@ -2880,6 +2879,14 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
 													 subpath->pathkeys,
 													 &presorted_keys);
 
+			/*
+			 * When the partial path is already sorted, we can just add a gather
+			 * merge on top, and we're done - no point in adding explicit sort.
+			 *
+			 * XXX Can't we skip this (maybe only for the cheapest partial path)
+			 * when the path is already sorted? Then it's likely duplicate with
+			 * the path created by generate_gather_paths.
+			 */
 			if (is_sorted)
 			{
 				path = create_gather_merge_path(root, rel, subpath, rel->reltarget,
@@ -2892,8 +2899,14 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
 			Assert(!is_sorted);
 
 			/*
-			 * consider regular sort for cheapest partial path (for each
-			 * useful pathkeys)
+			 * Consider regular sort for the cheapest partial path (for each
+			 * useful pathkeys). We know the path is not sorted, because we'd
+			 * not get here otherwise.
+			 *
+			 * XXX This is not redundant with the gather merge path created in
+			 * generate_gather_paths, because that merely preserves ordering of
+			 * the cheapest partial path, while here we add an explicit sort to
+			 * get match the useful ordering.
 			 */
 			if (cheapest_partial_path == subpath)
 			{
@@ -2919,7 +2932,10 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
 				/* Fall through */
 			}
 
-			/* Also consider incremental sort */
+			/*
+			 * Consider incremental sort, but only when the subpath is already
+			 * partially sorted on a pathkey prefix.
+			 */
 			if (presorted_keys > 0)
 			{
 				Path	   *tmp;
-- 
2.21.1


--dfcjsgdukgytabqd
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v39-0007-A-couple-more-places-for-incremental-sort.patch"



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* [PATCH 8/8] fix
@ 2020-03-19 17:48  Tomas Vondra <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Tomas Vondra @ 2020-03-19 17:48 UTC (permalink / raw)

---
 src/backend/optimizer/plan/planner.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 2880fcabe8..b4763e79f8 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5100,17 +5100,17 @@ create_ordered_paths(PlannerInfo *root,
 				double		total_groups;
 
 				/*
-				 * We don't care if this is the cheapest partial path - we
-				 * can't simply skip it, because it may be partially sorted in
-				 * which case we want to consider incremental sort on top of
-				 * it (instead of full sort, which is what happens above).
+				 * We don't care if this is the cheapest partial path - we can't
+				 * simply skip it, because it may be partially sorted in which
+				 * case we want to consider adding incremental sort (instead of
+				 * full sort, which is what happens above).
 				 */
 
 				is_sorted = pathkeys_common_contained_in(root->sort_pathkeys,
 														 input_path->pathkeys,
 														 &presorted_keys);
 
-				/* Ignore already sorted paths */
+				/* No point in adding incremental sort on fully sorted paths. */
 				if (is_sorted)
 					continue;
 
@@ -7005,9 +7005,7 @@ create_partial_grouping_paths(PlannerInfo *root,
 			}
 		}
 
-		/*
-		 * Also consider incremental sort on all partially sorted paths.
-		 */
+		/* Consider incremental sort on all partial paths, if enabled. */
 		if (enable_incrementalsort)
 		{
 			foreach(lc, input_rel->pathlist)
@@ -7122,10 +7120,10 @@ create_partial_grouping_paths(PlannerInfo *root,
 			/* We've already skipped fully sorted paths above. */
 			Assert(!is_sorted);
 
+			/* no shared prefix, not point in building incremental sort */
 			if (presorted_keys == 0)
 				continue;
 
-			/* Since we have presorted keys, consider incremental sort. */
 			path = (Path *) create_incremental_sort_path(root,
 														 partially_grouped_rel,
 														 path,
-- 
2.21.1


--dfcjsgdukgytabqd--





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Allow tailoring of ICU locales with custom rules
@ 2022-12-14 09:26  Peter Eisentraut <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Peter Eisentraut @ 2022-12-14 09:26 UTC (permalink / raw)
  To: pgsql-hackers

This patch exposes the ICU facility to add custom collation rules to a 
standard collation.  This would allow users to customize any ICU 
collation to whatever they want.  A very simple example from the 
documentation/tests:

CREATE COLLATION en_custom
     (provider = icu, locale = 'en', rules = '&a < g');

This places "g" after "a" before "b".  Details about the syntax can be 
found at 
<https://unicode-org.github.io/icu/userguide/collation/customization/;.

The code is pretty straightforward.  It mainly just records these rules 
in the catalog and feeds them to ICU when creating the collator object.
From b0d42407a60e116d3ccb0ed04505aa362f8a6a1d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 14 Dec 2022 10:15:03 +0100
Subject: [PATCH] Allow tailoring of ICU locales with custom rules

This exposes the ICU facility to add custom collation rules to a
standard collation.
---
 doc/src/sgml/catalogs.sgml                    | 18 +++++++
 doc/src/sgml/ref/create_collation.sgml        | 22 +++++++++
 doc/src/sgml/ref/create_database.sgml         | 12 +++++
 src/backend/catalog/pg_collation.c            |  5 ++
 src/backend/commands/collationcmds.c          | 21 ++++++--
 src/backend/commands/dbcommands.c             | 49 +++++++++++++++++--
 src/backend/utils/adt/pg_locale.c             | 41 +++++++++++++++-
 src/backend/utils/init/postinit.c             | 11 ++++-
 src/include/catalog/pg_collation.h            |  2 +
 src/include/catalog/pg_database.h             |  3 ++
 src/include/utils/pg_locale.h                 |  1 +
 .../regress/expected/collate.icu.utf8.out     | 30 ++++++++++++
 src/test/regress/sql/collate.icu.utf8.sql     | 13 +++++
 13 files changed, 219 insertions(+), 9 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 9316b811ac..afa9f28ef9 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2428,6 +2428,15 @@ <title><structname>pg_collation</structname> Columns</title>
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>collicurules</structfield> <type>text</type>
+      </para>
+      <para>
+       ICU collation rules for this collation object
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>collversion</structfield> <type>text</type>
@@ -3106,6 +3115,15 @@ <title><structname>pg_database</structname> Columns</title>
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>daticurules</structfield> <type>text</type>
+      </para>
+      <para>
+       ICU collation rules for this database
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>datcollversion</structfield> <type>text</type>
diff --git a/doc/src/sgml/ref/create_collation.sgml b/doc/src/sgml/ref/create_collation.sgml
index 58f5f0cd63..2c7266107e 100644
--- a/doc/src/sgml/ref/create_collation.sgml
+++ b/doc/src/sgml/ref/create_collation.sgml
@@ -27,6 +27,7 @@
     [ LC_CTYPE = <replaceable>lc_ctype</replaceable>, ]
     [ PROVIDER = <replaceable>provider</replaceable>, ]
     [ DETERMINISTIC = <replaceable>boolean</replaceable>, ]
+    [ RULES = <replaceable>rules</replaceable>, ]
     [ VERSION = <replaceable>version</replaceable> ]
 )
 CREATE COLLATION [ IF NOT EXISTS ] <replaceable>name</replaceable> FROM <replaceable>existing_collation</replaceable>
@@ -149,6 +150,19 @@ <title>Parameters</title>
      </listitem>
     </varlistentry>
 
+    <varlistentry>
+     <term><replaceable>rules</replaceable></term>
+
+     <listitem>
+      <para>
+       Specifies additional collation rules to customize the behavior of the
+       collation.  This is supported for ICU only.  See <ulink
+       url="https://unicode-org.github.io/icu/userguide/collation/customization/"/;
+       for details on the syntax.
+      </para>
+     </listitem>
+    </varlistentry>
+
     <varlistentry>
      <term><replaceable>version</replaceable></term>
 
@@ -228,6 +242,14 @@ <title>Examples</title>
 </programlisting>
   </para>
 
+  <para>
+   To create a collation using the ICU provider, based on the English ICU
+   locale, with custom rules:
+<programlisting>
+<![CDATA[CREATE COLLATION en_custom (provider = icu, locale = 'en', rules = '&a < g');]]>
+</programlisting>
+  </para>
+
   <para>
    To create a collation from an existing collation:
 <programlisting>
diff --git a/doc/src/sgml/ref/create_database.sgml b/doc/src/sgml/ref/create_database.sgml
index ea38c64731..aa6f121a81 100644
--- a/doc/src/sgml/ref/create_database.sgml
+++ b/doc/src/sgml/ref/create_database.sgml
@@ -192,6 +192,18 @@ <title>Parameters</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><replaceable class="parameter">icu_rules</replaceable></term>
+      <listitem>
+       <para>
+        Specifies additional collation rules to customize the behavior of the
+        collation.  This is supported for ICU only.  See <ulink
+        url="https://unicode-org.github.io/icu/userguide/collation/customization/"/;
+        for details on the syntax.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><replaceable>locale_provider</replaceable></term>
 
diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c
index aa8fbe1a98..7ed9de3891 100644
--- a/src/backend/catalog/pg_collation.c
+++ b/src/backend/catalog/pg_collation.c
@@ -50,6 +50,7 @@ CollationCreate(const char *collname, Oid collnamespace,
 				int32 collencoding,
 				const char *collcollate, const char *collctype,
 				const char *colliculocale,
+				const char *collicurules,
 				const char *collversion,
 				bool if_not_exists,
 				bool quiet)
@@ -194,6 +195,10 @@ CollationCreate(const char *collname, Oid collnamespace,
 		values[Anum_pg_collation_colliculocale - 1] = CStringGetTextDatum(colliculocale);
 	else
 		nulls[Anum_pg_collation_colliculocale - 1] = true;
+	if (collicurules)
+		values[Anum_pg_collation_collicurules - 1] = CStringGetTextDatum(collicurules);
+	else
+		nulls[Anum_pg_collation_collicurules - 1] = true;
 	if (collversion)
 		values[Anum_pg_collation_collversion - 1] = CStringGetTextDatum(collversion);
 	else
diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c
index 81e54e0ce6..50f16f2764 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -64,10 +64,12 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 	DefElem    *lcctypeEl = NULL;
 	DefElem    *providerEl = NULL;
 	DefElem    *deterministicEl = NULL;
+	DefElem    *rulesEl = NULL;
 	DefElem    *versionEl = NULL;
 	char	   *collcollate;
 	char	   *collctype;
 	char	   *colliculocale;
+	char	   *collicurules;
 	bool		collisdeterministic;
 	int			collencoding;
 	char		collprovider;
@@ -99,6 +101,8 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 			defelp = &providerEl;
 		else if (strcmp(defel->defname, "deterministic") == 0)
 			defelp = &deterministicEl;
+		else if (strcmp(defel->defname, "rules") == 0)
+			defelp = &rulesEl;
 		else if (strcmp(defel->defname, "version") == 0)
 			defelp = &versionEl;
 		else
@@ -161,6 +165,12 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 		else
 			colliculocale = NULL;
 
+		datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collicurules, &isnull);
+		if (!isnull)
+			collicurules = TextDatumGetCString(datum);
+		else
+			collicurules = NULL;
+
 		ReleaseSysCache(tp);
 
 		/*
@@ -182,6 +192,7 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 		collcollate = NULL;
 		collctype = NULL;
 		colliculocale = NULL;
+		collicurules = NULL;
 
 		if (providerEl)
 			collproviderstr = defGetString(providerEl);
@@ -191,6 +202,9 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 		else
 			collisdeterministic = true;
 
+		if (rulesEl)
+			collicurules = defGetString(rulesEl);
+
 		if (versionEl)
 			collversion = defGetString(versionEl);
 
@@ -297,6 +311,7 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 							 collcollate,
 							 collctype,
 							 colliculocale,
+							 collicurules,
 							 collversion,
 							 if_not_exists,
 							 false);	/* not quiet */
@@ -710,7 +725,7 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
 			 */
 			collid = CollationCreate(localebuf, nspid, GetUserId(),
 									 COLLPROVIDER_LIBC, true, enc,
-									 localebuf, localebuf, NULL,
+									 localebuf, localebuf, NULL, NULL,
 									 get_collation_actual_version(COLLPROVIDER_LIBC, localebuf),
 									 true, true);
 			if (OidIsValid(collid))
@@ -777,7 +792,7 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
 
 			collid = CollationCreate(alias, nspid, GetUserId(),
 									 COLLPROVIDER_LIBC, true, enc,
-									 locale, locale, NULL,
+									 locale, locale, NULL, NULL,
 									 get_collation_actual_version(COLLPROVIDER_LIBC, locale),
 									 true, true);
 			if (OidIsValid(collid))
@@ -839,7 +854,7 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
 			collid = CollationCreate(psprintf("%s-x-icu", langtag),
 									 nspid, GetUserId(),
 									 COLLPROVIDER_ICU, true, -1,
-									 NULL, NULL, iculocstr,
+									 NULL, NULL, iculocstr, NULL,
 									 get_collation_actual_version(COLLPROVIDER_ICU, iculocstr),
 									 true, true);
 			if (OidIsValid(collid))
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 6eb8742718..65d6ad46ef 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -119,6 +119,7 @@ static bool get_db_info(const char *name, LOCKMODE lockmode,
 						int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
 						TransactionId *dbFrozenXidP, MultiXactId *dbMinMultiP,
 						Oid *dbTablespace, char **dbCollate, char **dbCtype, char **dbIculocale,
+						char **dbIcurules,
 						char *dbLocProvider,
 						char **dbCollversion);
 static bool have_createdb_privilege(void);
@@ -676,6 +677,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	char	   *src_collate = NULL;
 	char	   *src_ctype = NULL;
 	char	   *src_iculocale = NULL;
+	char	   *src_icurules = NULL;
 	char		src_locprovider = '\0';
 	char	   *src_collversion = NULL;
 	bool		src_istemplate;
@@ -699,6 +701,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	DefElem    *dcollate = NULL;
 	DefElem    *dctype = NULL;
 	DefElem    *diculocale = NULL;
+	DefElem    *dicurules = NULL;
 	DefElem    *dlocprovider = NULL;
 	DefElem    *distemplate = NULL;
 	DefElem    *dallowconnections = NULL;
@@ -711,6 +714,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	char	   *dbcollate = NULL;
 	char	   *dbctype = NULL;
 	char	   *dbiculocale = NULL;
+	char	   *dbicurules = NULL;
 	char		dblocprovider = '\0';
 	char	   *canonname;
 	int			encoding = -1;
@@ -776,6 +780,12 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 				errorConflictingDefElem(defel, pstate);
 			diculocale = defel;
 		}
+		else if (strcmp(defel->defname, "icu_rules") == 0)
+		{
+			if (dicurules)
+				errorConflictingDefElem(defel, pstate);
+			dicurules = defel;
+		}
 		else if (strcmp(defel->defname, "locale_provider") == 0)
 		{
 			if (dlocprovider)
@@ -959,7 +969,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 					 &src_dboid, &src_owner, &src_encoding,
 					 &src_istemplate, &src_allowconn,
 					 &src_frozenxid, &src_minmxid, &src_deftablespace,
-					 &src_collate, &src_ctype, &src_iculocale, &src_locprovider,
+					 &src_collate, &src_ctype, &src_iculocale, &src_icurules, &src_locprovider,
 					 &src_collversion))
 		ereport(ERROR,
 				(errcode(ERRCODE_UNDEFINED_DATABASE),
@@ -1007,6 +1017,8 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 		dblocprovider = src_locprovider;
 	if (dbiculocale == NULL && dblocprovider == COLLPROVIDER_ICU)
 		dbiculocale = src_iculocale;
+	if (dbicurules == NULL && dblocprovider == COLLPROVIDER_ICU)
+		dbicurules = src_icurules;
 
 	/* Some encodings are client only */
 	if (!PG_VALID_BE_ENCODING(encoding))
@@ -1098,6 +1110,9 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 
 		if (dblocprovider == COLLPROVIDER_ICU)
 		{
+			char	   *val1;
+			char	   *val2;
+
 			Assert(dbiculocale);
 			Assert(src_iculocale);
 			if (strcmp(dbiculocale, src_iculocale) != 0)
@@ -1106,6 +1121,19 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 						 errmsg("new ICU locale (%s) is incompatible with the ICU locale of the template database (%s)",
 								dbiculocale, src_iculocale),
 						 errhint("Use the same ICU locale as in the template database, or use template0 as template.")));
+
+			val1 = dbicurules;
+			if (!val1)
+				val1 = "";
+			val2 = src_icurules;
+			if (!val2)
+				val2 = "";
+			if (strcmp(val1, val2) != 0)
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("new ICU collation rules (%s) are incompatible with the ICU collation rules of the template database (%s)",
+								val1, val2),
+						 errhint("Use the same ICU collation rules as in the template database, or use template0 as template.")));
 		}
 	}
 
@@ -1314,6 +1342,10 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 		new_record[Anum_pg_database_daticulocale - 1] = CStringGetTextDatum(dbiculocale);
 	else
 		new_record_nulls[Anum_pg_database_daticulocale - 1] = true;
+	if (dbicurules)
+		new_record[Anum_pg_database_daticurules - 1] = CStringGetTextDatum(dbicurules);
+	else
+		new_record_nulls[Anum_pg_database_daticurules - 1] = true;
 	if (dbcollversion)
 		new_record[Anum_pg_database_datcollversion - 1] = CStringGetTextDatum(dbcollversion);
 	else
@@ -1527,7 +1559,7 @@ dropdb(const char *dbname, bool missing_ok, bool force)
 	pgdbrel = table_open(DatabaseRelationId, RowExclusiveLock);
 
 	if (!get_db_info(dbname, AccessExclusiveLock, &db_id, NULL, NULL,
-					 &db_istemplate, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL))
+					 &db_istemplate, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL))
 	{
 		if (!missing_ok)
 		{
@@ -1727,7 +1759,7 @@ RenameDatabase(const char *oldname, const char *newname)
 	rel = table_open(DatabaseRelationId, RowExclusiveLock);
 
 	if (!get_db_info(oldname, AccessExclusiveLock, &db_id, NULL, NULL,
-					 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL))
+					 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL))
 		ereport(ERROR,
 				(errcode(ERRCODE_UNDEFINED_DATABASE),
 				 errmsg("database \"%s\" does not exist", oldname)));
@@ -1837,7 +1869,7 @@ movedb(const char *dbname, const char *tblspcname)
 	pgdbrel = table_open(DatabaseRelationId, RowExclusiveLock);
 
 	if (!get_db_info(dbname, AccessExclusiveLock, &db_id, NULL, NULL,
-					 NULL, NULL, NULL, NULL, &src_tblspcoid, NULL, NULL, NULL, NULL, NULL))
+					 NULL, NULL, NULL, NULL, &src_tblspcoid, NULL, NULL, NULL, NULL, NULL, NULL))
 		ereport(ERROR,
 				(errcode(ERRCODE_UNDEFINED_DATABASE),
 				 errmsg("database \"%s\" does not exist", dbname)));
@@ -2600,6 +2632,7 @@ get_db_info(const char *name, LOCKMODE lockmode,
 			int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
 			TransactionId *dbFrozenXidP, MultiXactId *dbMinMultiP,
 			Oid *dbTablespace, char **dbCollate, char **dbCtype, char **dbIculocale,
+			char **dbIcurules,
 			char *dbLocProvider,
 			char **dbCollversion)
 {
@@ -2716,6 +2749,14 @@ get_db_info(const char *name, LOCKMODE lockmode,
 					else
 						*dbIculocale = TextDatumGetCString(datum);
 				}
+				if (dbIcurules)
+				{
+					datum = SysCacheGetAttr(DATABASEOID, tuple, Anum_pg_database_daticurules, &isnull);
+					if (isnull)
+						*dbIcurules = NULL;
+					else
+						*dbIcurules = TextDatumGetCString(datum);
+				}
 				if (dbCollversion)
 				{
 					datum = SysCacheGetAttr(DATABASEOID, tuple, Anum_pg_database_datcollversion, &isnull);
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 2b42d9ccd8..191d2e8a68 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -69,6 +69,7 @@
 
 #ifdef USE_ICU
 #include <unicode/ucnv.h>
+#include <unicode/ustring.h>
 #endif
 
 #ifdef __GLIBC__
@@ -1402,6 +1403,7 @@ struct pg_locale_struct default_locale;
 
 void
 make_icu_collator(const char *iculocstr,
+				  const char *icurules,
 				  struct pg_locale_struct *resultp)
 {
 #ifdef USE_ICU
@@ -1418,6 +1420,35 @@ make_icu_collator(const char *iculocstr,
 	if (U_ICU_VERSION_MAJOR_NUM < 54)
 		icu_set_collation_attributes(collator, iculocstr);
 
+	/*
+	 * If rules are specified, we extract the rules of the standard collation,
+	 * add our own rules, and make a new collator with the combined rules.
+	 */
+	if (icurules)
+	{
+		const UChar *default_rules;
+		UChar	   *agg_rules;
+		UChar	   *my_rules;
+		int32_t		length;
+
+		default_rules = ucol_getRules(collator, &length);
+		icu_to_uchar(&my_rules, icurules, strlen(icurules));
+
+		agg_rules = palloc_array(UChar, u_strlen(default_rules) + u_strlen(my_rules) + 1);
+		u_strcpy(agg_rules, default_rules);
+		u_strcat(agg_rules, my_rules);
+
+		ucol_close(collator);
+
+		status = U_ZERO_ERROR;
+		collator = ucol_openRules(agg_rules, u_strlen(agg_rules),
+								  UCOL_DEFAULT, UCOL_DEFAULT_STRENGTH, NULL, &status);
+		if (U_FAILURE(status))
+			ereport(ERROR,
+					(errmsg("could not open collator for locale \"%s\" with rules \"%s\": %s",
+							iculocstr, icurules, u_errorName(status))));
+	}
+
 	/* We will leak this string if the caller errors later :-( */
 	resultp->info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr);
 	resultp->info.icu.ucol = collator;
@@ -1580,11 +1611,19 @@ pg_newlocale_from_collation(Oid collid)
 		else if (collform->collprovider == COLLPROVIDER_ICU)
 		{
 			const char *iculocstr;
+			const char *icurules;
 
 			datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_colliculocale, &isnull);
 			Assert(!isnull);
 			iculocstr = TextDatumGetCString(datum);
-			make_icu_collator(iculocstr, &result);
+
+			datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collicurules, &isnull);
+			if (!isnull)
+				icurules = TextDatumGetCString(datum);
+			else
+				icurules = NULL;
+
+			make_icu_collator(iculocstr, icurules, &result);
 		}
 
 		datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collversion,
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index a990c833c5..55a035c062 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -421,10 +421,19 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
 
 	if (dbform->datlocprovider == COLLPROVIDER_ICU)
 	{
+		char	   *icurules;
+
 		datum = SysCacheGetAttr(DATABASEOID, tup, Anum_pg_database_daticulocale, &isnull);
 		Assert(!isnull);
 		iculocale = TextDatumGetCString(datum);
-		make_icu_collator(iculocale, &default_locale);
+
+		datum = SysCacheGetAttr(DATABASEOID, tup, Anum_pg_database_daticurules, &isnull);
+		if (!isnull)
+			icurules = TextDatumGetCString(datum);
+		else
+			icurules = NULL;
+
+		make_icu_collator(iculocale, icurules, &default_locale);
 	}
 	else
 		iculocale = NULL;
diff --git a/src/include/catalog/pg_collation.h b/src/include/catalog/pg_collation.h
index 2190ccb5b8..ad2d767d65 100644
--- a/src/include/catalog/pg_collation.h
+++ b/src/include/catalog/pg_collation.h
@@ -43,6 +43,7 @@ CATALOG(pg_collation,3456,CollationRelationId)
 	text		collcollate BKI_DEFAULT(_null_);	/* LC_COLLATE setting */
 	text		collctype BKI_DEFAULT(_null_);	/* LC_CTYPE setting */
 	text		colliculocale BKI_DEFAULT(_null_);	/* ICU locale ID */
+	text		collicurules BKI_DEFAULT(_null_);	/* ICU collation rules */
 	text		collversion BKI_DEFAULT(_null_);	/* provider-dependent
 													 * version of collation
 													 * data */
@@ -91,6 +92,7 @@ extern Oid	CollationCreate(const char *collname, Oid collnamespace,
 							int32 collencoding,
 							const char *collcollate, const char *collctype,
 							const char *colliculocale,
+							const char *collicurules,
 							const char *collversion,
 							bool if_not_exists,
 							bool quiet);
diff --git a/src/include/catalog/pg_database.h b/src/include/catalog/pg_database.h
index 611c95656a..48bc285863 100644
--- a/src/include/catalog/pg_database.h
+++ b/src/include/catalog/pg_database.h
@@ -71,6 +71,9 @@ CATALOG(pg_database,1262,DatabaseRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID
 	/* ICU locale ID */
 	text		daticulocale;
 
+	/* ICU collation rules */
+	text		daticurules BKI_DEFAULT(_null_);
+
 	/* provider-dependent version of collation data */
 	text		datcollversion BKI_DEFAULT(_null_);
 
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index a875942123..989fe26cef 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -95,6 +95,7 @@ typedef struct pg_locale_struct *pg_locale_t;
 extern PGDLLIMPORT struct pg_locale_struct default_locale;
 
 extern void make_icu_collator(const char *iculocstr,
+							  const char *icurules,
 							  struct pg_locale_struct *resultp);
 
 extern pg_locale_t pg_newlocale_from_collation(Oid collid);
diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out
index d4c8c6de38..ce9ed3c8bf 100644
--- a/src/test/regress/expected/collate.icu.utf8.out
+++ b/src/test/regress/expected/collate.icu.utf8.out
@@ -1190,6 +1190,36 @@ SELECT 'Goldmann' < 'Götz' COLLATE "de-x-icu", 'Goldmann' > 'Götz' COLLATE tes
  t        | t
 (1 row)
 
+-- rules
+CREATE COLLATION testcoll_rules1 (provider = icu, locale = '', rules = '&a < g');
+CREATE TABLE test7 (a text);
+-- example from https://unicode-org.github.io/icu/userguide/collation/customization/#syntax
+INSERT INTO test7 VALUES ('Abernathy'), ('apple'), ('bird'), ('Boston'), ('Graham'), ('green');
+SELECT * FROM test7 ORDER BY a COLLATE "en-x-icu";
+     a     
+-----------
+ Abernathy
+ apple
+ bird
+ Boston
+ Graham
+ green
+(6 rows)
+
+SELECT * FROM test7 ORDER BY a COLLATE testcoll_rules1;
+     a     
+-----------
+ Abernathy
+ apple
+ green
+ bird
+ Boston
+ Graham
+(6 rows)
+
+DROP TABLE test7;
+CREATE COLLATION testcoll_rulesx (provider = icu, locale = '', rules = '!!wrong!!');
+ERROR:  could not open collator for locale "" with rules "!!wrong!!": U_INVALID_FORMAT_ERROR
 -- nondeterministic collations
 CREATE COLLATION ctest_det (provider = icu, locale = '', deterministic = true);
 CREATE COLLATION ctest_nondet (provider = icu, locale = '', deterministic = false);
diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql
index b0ddc7db44..aa95c1ec42 100644
--- a/src/test/regress/sql/collate.icu.utf8.sql
+++ b/src/test/regress/sql/collate.icu.utf8.sql
@@ -472,6 +472,19 @@ CREATE COLLATION testcoll_de_phonebook (provider = icu, locale = 'de@collation=p
 SELECT 'Goldmann' < 'Götz' COLLATE "de-x-icu", 'Goldmann' > 'Götz' COLLATE testcoll_de_phonebook;
 
 
+-- rules
+
+CREATE COLLATION testcoll_rules1 (provider = icu, locale = '', rules = '&a < g');
+CREATE TABLE test7 (a text);
+-- example from https://unicode-org.github.io/icu/userguide/collation/customization/#syntax
+INSERT INTO test7 VALUES ('Abernathy'), ('apple'), ('bird'), ('Boston'), ('Graham'), ('green');
+SELECT * FROM test7 ORDER BY a COLLATE "en-x-icu";
+SELECT * FROM test7 ORDER BY a COLLATE testcoll_rules1;
+DROP TABLE test7;
+
+CREATE COLLATION testcoll_rulesx (provider = icu, locale = '', rules = '!!wrong!!');
+
+
 -- nondeterministic collations
 
 CREATE COLLATION ctest_det (provider = icu, locale = '', deterministic = true);
-- 
2.38.1



Attachments:

  [text/plain] 0001-Allow-tailoring-of-ICU-locales-with-custom-rules.patch (22.9K, ../../[email protected]/2-0001-Allow-tailoring-of-ICU-locales-with-custom-rules.patch)
  download | inline diff:
From b0d42407a60e116d3ccb0ed04505aa362f8a6a1d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 14 Dec 2022 10:15:03 +0100
Subject: [PATCH] Allow tailoring of ICU locales with custom rules

This exposes the ICU facility to add custom collation rules to a
standard collation.
---
 doc/src/sgml/catalogs.sgml                    | 18 +++++++
 doc/src/sgml/ref/create_collation.sgml        | 22 +++++++++
 doc/src/sgml/ref/create_database.sgml         | 12 +++++
 src/backend/catalog/pg_collation.c            |  5 ++
 src/backend/commands/collationcmds.c          | 21 ++++++--
 src/backend/commands/dbcommands.c             | 49 +++++++++++++++++--
 src/backend/utils/adt/pg_locale.c             | 41 +++++++++++++++-
 src/backend/utils/init/postinit.c             | 11 ++++-
 src/include/catalog/pg_collation.h            |  2 +
 src/include/catalog/pg_database.h             |  3 ++
 src/include/utils/pg_locale.h                 |  1 +
 .../regress/expected/collate.icu.utf8.out     | 30 ++++++++++++
 src/test/regress/sql/collate.icu.utf8.sql     | 13 +++++
 13 files changed, 219 insertions(+), 9 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 9316b811ac..afa9f28ef9 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2428,6 +2428,15 @@ <title><structname>pg_collation</structname> Columns</title>
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>collicurules</structfield> <type>text</type>
+      </para>
+      <para>
+       ICU collation rules for this collation object
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>collversion</structfield> <type>text</type>
@@ -3106,6 +3115,15 @@ <title><structname>pg_database</structname> Columns</title>
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>daticurules</structfield> <type>text</type>
+      </para>
+      <para>
+       ICU collation rules for this database
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>datcollversion</structfield> <type>text</type>
diff --git a/doc/src/sgml/ref/create_collation.sgml b/doc/src/sgml/ref/create_collation.sgml
index 58f5f0cd63..2c7266107e 100644
--- a/doc/src/sgml/ref/create_collation.sgml
+++ b/doc/src/sgml/ref/create_collation.sgml
@@ -27,6 +27,7 @@
     [ LC_CTYPE = <replaceable>lc_ctype</replaceable>, ]
     [ PROVIDER = <replaceable>provider</replaceable>, ]
     [ DETERMINISTIC = <replaceable>boolean</replaceable>, ]
+    [ RULES = <replaceable>rules</replaceable>, ]
     [ VERSION = <replaceable>version</replaceable> ]
 )
 CREATE COLLATION [ IF NOT EXISTS ] <replaceable>name</replaceable> FROM <replaceable>existing_collation</replaceable>
@@ -149,6 +150,19 @@ <title>Parameters</title>
      </listitem>
     </varlistentry>
 
+    <varlistentry>
+     <term><replaceable>rules</replaceable></term>
+
+     <listitem>
+      <para>
+       Specifies additional collation rules to customize the behavior of the
+       collation.  This is supported for ICU only.  See <ulink
+       url="https://unicode-org.github.io/icu/userguide/collation/customization/"/>
+       for details on the syntax.
+      </para>
+     </listitem>
+    </varlistentry>
+
     <varlistentry>
      <term><replaceable>version</replaceable></term>
 
@@ -228,6 +242,14 @@ <title>Examples</title>
 </programlisting>
   </para>
 
+  <para>
+   To create a collation using the ICU provider, based on the English ICU
+   locale, with custom rules:
+<programlisting>
+<![CDATA[CREATE COLLATION en_custom (provider = icu, locale = 'en', rules = '&a < g');]]>
+</programlisting>
+  </para>
+
   <para>
    To create a collation from an existing collation:
 <programlisting>
diff --git a/doc/src/sgml/ref/create_database.sgml b/doc/src/sgml/ref/create_database.sgml
index ea38c64731..aa6f121a81 100644
--- a/doc/src/sgml/ref/create_database.sgml
+++ b/doc/src/sgml/ref/create_database.sgml
@@ -192,6 +192,18 @@ <title>Parameters</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><replaceable class="parameter">icu_rules</replaceable></term>
+      <listitem>
+       <para>
+        Specifies additional collation rules to customize the behavior of the
+        collation.  This is supported for ICU only.  See <ulink
+        url="https://unicode-org.github.io/icu/userguide/collation/customization/"/>
+        for details on the syntax.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><replaceable>locale_provider</replaceable></term>
 
diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c
index aa8fbe1a98..7ed9de3891 100644
--- a/src/backend/catalog/pg_collation.c
+++ b/src/backend/catalog/pg_collation.c
@@ -50,6 +50,7 @@ CollationCreate(const char *collname, Oid collnamespace,
 				int32 collencoding,
 				const char *collcollate, const char *collctype,
 				const char *colliculocale,
+				const char *collicurules,
 				const char *collversion,
 				bool if_not_exists,
 				bool quiet)
@@ -194,6 +195,10 @@ CollationCreate(const char *collname, Oid collnamespace,
 		values[Anum_pg_collation_colliculocale - 1] = CStringGetTextDatum(colliculocale);
 	else
 		nulls[Anum_pg_collation_colliculocale - 1] = true;
+	if (collicurules)
+		values[Anum_pg_collation_collicurules - 1] = CStringGetTextDatum(collicurules);
+	else
+		nulls[Anum_pg_collation_collicurules - 1] = true;
 	if (collversion)
 		values[Anum_pg_collation_collversion - 1] = CStringGetTextDatum(collversion);
 	else
diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c
index 81e54e0ce6..50f16f2764 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -64,10 +64,12 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 	DefElem    *lcctypeEl = NULL;
 	DefElem    *providerEl = NULL;
 	DefElem    *deterministicEl = NULL;
+	DefElem    *rulesEl = NULL;
 	DefElem    *versionEl = NULL;
 	char	   *collcollate;
 	char	   *collctype;
 	char	   *colliculocale;
+	char	   *collicurules;
 	bool		collisdeterministic;
 	int			collencoding;
 	char		collprovider;
@@ -99,6 +101,8 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 			defelp = &providerEl;
 		else if (strcmp(defel->defname, "deterministic") == 0)
 			defelp = &deterministicEl;
+		else if (strcmp(defel->defname, "rules") == 0)
+			defelp = &rulesEl;
 		else if (strcmp(defel->defname, "version") == 0)
 			defelp = &versionEl;
 		else
@@ -161,6 +165,12 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 		else
 			colliculocale = NULL;
 
+		datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collicurules, &isnull);
+		if (!isnull)
+			collicurules = TextDatumGetCString(datum);
+		else
+			collicurules = NULL;
+
 		ReleaseSysCache(tp);
 
 		/*
@@ -182,6 +192,7 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 		collcollate = NULL;
 		collctype = NULL;
 		colliculocale = NULL;
+		collicurules = NULL;
 
 		if (providerEl)
 			collproviderstr = defGetString(providerEl);
@@ -191,6 +202,9 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 		else
 			collisdeterministic = true;
 
+		if (rulesEl)
+			collicurules = defGetString(rulesEl);
+
 		if (versionEl)
 			collversion = defGetString(versionEl);
 
@@ -297,6 +311,7 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 							 collcollate,
 							 collctype,
 							 colliculocale,
+							 collicurules,
 							 collversion,
 							 if_not_exists,
 							 false);	/* not quiet */
@@ -710,7 +725,7 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
 			 */
 			collid = CollationCreate(localebuf, nspid, GetUserId(),
 									 COLLPROVIDER_LIBC, true, enc,
-									 localebuf, localebuf, NULL,
+									 localebuf, localebuf, NULL, NULL,
 									 get_collation_actual_version(COLLPROVIDER_LIBC, localebuf),
 									 true, true);
 			if (OidIsValid(collid))
@@ -777,7 +792,7 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
 
 			collid = CollationCreate(alias, nspid, GetUserId(),
 									 COLLPROVIDER_LIBC, true, enc,
-									 locale, locale, NULL,
+									 locale, locale, NULL, NULL,
 									 get_collation_actual_version(COLLPROVIDER_LIBC, locale),
 									 true, true);
 			if (OidIsValid(collid))
@@ -839,7 +854,7 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
 			collid = CollationCreate(psprintf("%s-x-icu", langtag),
 									 nspid, GetUserId(),
 									 COLLPROVIDER_ICU, true, -1,
-									 NULL, NULL, iculocstr,
+									 NULL, NULL, iculocstr, NULL,
 									 get_collation_actual_version(COLLPROVIDER_ICU, iculocstr),
 									 true, true);
 			if (OidIsValid(collid))
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 6eb8742718..65d6ad46ef 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -119,6 +119,7 @@ static bool get_db_info(const char *name, LOCKMODE lockmode,
 						int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
 						TransactionId *dbFrozenXidP, MultiXactId *dbMinMultiP,
 						Oid *dbTablespace, char **dbCollate, char **dbCtype, char **dbIculocale,
+						char **dbIcurules,
 						char *dbLocProvider,
 						char **dbCollversion);
 static bool have_createdb_privilege(void);
@@ -676,6 +677,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	char	   *src_collate = NULL;
 	char	   *src_ctype = NULL;
 	char	   *src_iculocale = NULL;
+	char	   *src_icurules = NULL;
 	char		src_locprovider = '\0';
 	char	   *src_collversion = NULL;
 	bool		src_istemplate;
@@ -699,6 +701,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	DefElem    *dcollate = NULL;
 	DefElem    *dctype = NULL;
 	DefElem    *diculocale = NULL;
+	DefElem    *dicurules = NULL;
 	DefElem    *dlocprovider = NULL;
 	DefElem    *distemplate = NULL;
 	DefElem    *dallowconnections = NULL;
@@ -711,6 +714,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	char	   *dbcollate = NULL;
 	char	   *dbctype = NULL;
 	char	   *dbiculocale = NULL;
+	char	   *dbicurules = NULL;
 	char		dblocprovider = '\0';
 	char	   *canonname;
 	int			encoding = -1;
@@ -776,6 +780,12 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 				errorConflictingDefElem(defel, pstate);
 			diculocale = defel;
 		}
+		else if (strcmp(defel->defname, "icu_rules") == 0)
+		{
+			if (dicurules)
+				errorConflictingDefElem(defel, pstate);
+			dicurules = defel;
+		}
 		else if (strcmp(defel->defname, "locale_provider") == 0)
 		{
 			if (dlocprovider)
@@ -959,7 +969,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 					 &src_dboid, &src_owner, &src_encoding,
 					 &src_istemplate, &src_allowconn,
 					 &src_frozenxid, &src_minmxid, &src_deftablespace,
-					 &src_collate, &src_ctype, &src_iculocale, &src_locprovider,
+					 &src_collate, &src_ctype, &src_iculocale, &src_icurules, &src_locprovider,
 					 &src_collversion))
 		ereport(ERROR,
 				(errcode(ERRCODE_UNDEFINED_DATABASE),
@@ -1007,6 +1017,8 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 		dblocprovider = src_locprovider;
 	if (dbiculocale == NULL && dblocprovider == COLLPROVIDER_ICU)
 		dbiculocale = src_iculocale;
+	if (dbicurules == NULL && dblocprovider == COLLPROVIDER_ICU)
+		dbicurules = src_icurules;
 
 	/* Some encodings are client only */
 	if (!PG_VALID_BE_ENCODING(encoding))
@@ -1098,6 +1110,9 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 
 		if (dblocprovider == COLLPROVIDER_ICU)
 		{
+			char	   *val1;
+			char	   *val2;
+
 			Assert(dbiculocale);
 			Assert(src_iculocale);
 			if (strcmp(dbiculocale, src_iculocale) != 0)
@@ -1106,6 +1121,19 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 						 errmsg("new ICU locale (%s) is incompatible with the ICU locale of the template database (%s)",
 								dbiculocale, src_iculocale),
 						 errhint("Use the same ICU locale as in the template database, or use template0 as template.")));
+
+			val1 = dbicurules;
+			if (!val1)
+				val1 = "";
+			val2 = src_icurules;
+			if (!val2)
+				val2 = "";
+			if (strcmp(val1, val2) != 0)
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("new ICU collation rules (%s) are incompatible with the ICU collation rules of the template database (%s)",
+								val1, val2),
+						 errhint("Use the same ICU collation rules as in the template database, or use template0 as template.")));
 		}
 	}
 
@@ -1314,6 +1342,10 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 		new_record[Anum_pg_database_daticulocale - 1] = CStringGetTextDatum(dbiculocale);
 	else
 		new_record_nulls[Anum_pg_database_daticulocale - 1] = true;
+	if (dbicurules)
+		new_record[Anum_pg_database_daticurules - 1] = CStringGetTextDatum(dbicurules);
+	else
+		new_record_nulls[Anum_pg_database_daticurules - 1] = true;
 	if (dbcollversion)
 		new_record[Anum_pg_database_datcollversion - 1] = CStringGetTextDatum(dbcollversion);
 	else
@@ -1527,7 +1559,7 @@ dropdb(const char *dbname, bool missing_ok, bool force)
 	pgdbrel = table_open(DatabaseRelationId, RowExclusiveLock);
 
 	if (!get_db_info(dbname, AccessExclusiveLock, &db_id, NULL, NULL,
-					 &db_istemplate, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL))
+					 &db_istemplate, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL))
 	{
 		if (!missing_ok)
 		{
@@ -1727,7 +1759,7 @@ RenameDatabase(const char *oldname, const char *newname)
 	rel = table_open(DatabaseRelationId, RowExclusiveLock);
 
 	if (!get_db_info(oldname, AccessExclusiveLock, &db_id, NULL, NULL,
-					 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL))
+					 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL))
 		ereport(ERROR,
 				(errcode(ERRCODE_UNDEFINED_DATABASE),
 				 errmsg("database \"%s\" does not exist", oldname)));
@@ -1837,7 +1869,7 @@ movedb(const char *dbname, const char *tblspcname)
 	pgdbrel = table_open(DatabaseRelationId, RowExclusiveLock);
 
 	if (!get_db_info(dbname, AccessExclusiveLock, &db_id, NULL, NULL,
-					 NULL, NULL, NULL, NULL, &src_tblspcoid, NULL, NULL, NULL, NULL, NULL))
+					 NULL, NULL, NULL, NULL, &src_tblspcoid, NULL, NULL, NULL, NULL, NULL, NULL))
 		ereport(ERROR,
 				(errcode(ERRCODE_UNDEFINED_DATABASE),
 				 errmsg("database \"%s\" does not exist", dbname)));
@@ -2600,6 +2632,7 @@ get_db_info(const char *name, LOCKMODE lockmode,
 			int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
 			TransactionId *dbFrozenXidP, MultiXactId *dbMinMultiP,
 			Oid *dbTablespace, char **dbCollate, char **dbCtype, char **dbIculocale,
+			char **dbIcurules,
 			char *dbLocProvider,
 			char **dbCollversion)
 {
@@ -2716,6 +2749,14 @@ get_db_info(const char *name, LOCKMODE lockmode,
 					else
 						*dbIculocale = TextDatumGetCString(datum);
 				}
+				if (dbIcurules)
+				{
+					datum = SysCacheGetAttr(DATABASEOID, tuple, Anum_pg_database_daticurules, &isnull);
+					if (isnull)
+						*dbIcurules = NULL;
+					else
+						*dbIcurules = TextDatumGetCString(datum);
+				}
 				if (dbCollversion)
 				{
 					datum = SysCacheGetAttr(DATABASEOID, tuple, Anum_pg_database_datcollversion, &isnull);
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 2b42d9ccd8..191d2e8a68 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -69,6 +69,7 @@
 
 #ifdef USE_ICU
 #include <unicode/ucnv.h>
+#include <unicode/ustring.h>
 #endif
 
 #ifdef __GLIBC__
@@ -1402,6 +1403,7 @@ struct pg_locale_struct default_locale;
 
 void
 make_icu_collator(const char *iculocstr,
+				  const char *icurules,
 				  struct pg_locale_struct *resultp)
 {
 #ifdef USE_ICU
@@ -1418,6 +1420,35 @@ make_icu_collator(const char *iculocstr,
 	if (U_ICU_VERSION_MAJOR_NUM < 54)
 		icu_set_collation_attributes(collator, iculocstr);
 
+	/*
+	 * If rules are specified, we extract the rules of the standard collation,
+	 * add our own rules, and make a new collator with the combined rules.
+	 */
+	if (icurules)
+	{
+		const UChar *default_rules;
+		UChar	   *agg_rules;
+		UChar	   *my_rules;
+		int32_t		length;
+
+		default_rules = ucol_getRules(collator, &length);
+		icu_to_uchar(&my_rules, icurules, strlen(icurules));
+
+		agg_rules = palloc_array(UChar, u_strlen(default_rules) + u_strlen(my_rules) + 1);
+		u_strcpy(agg_rules, default_rules);
+		u_strcat(agg_rules, my_rules);
+
+		ucol_close(collator);
+
+		status = U_ZERO_ERROR;
+		collator = ucol_openRules(agg_rules, u_strlen(agg_rules),
+								  UCOL_DEFAULT, UCOL_DEFAULT_STRENGTH, NULL, &status);
+		if (U_FAILURE(status))
+			ereport(ERROR,
+					(errmsg("could not open collator for locale \"%s\" with rules \"%s\": %s",
+							iculocstr, icurules, u_errorName(status))));
+	}
+
 	/* We will leak this string if the caller errors later :-( */
 	resultp->info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr);
 	resultp->info.icu.ucol = collator;
@@ -1580,11 +1611,19 @@ pg_newlocale_from_collation(Oid collid)
 		else if (collform->collprovider == COLLPROVIDER_ICU)
 		{
 			const char *iculocstr;
+			const char *icurules;
 
 			datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_colliculocale, &isnull);
 			Assert(!isnull);
 			iculocstr = TextDatumGetCString(datum);
-			make_icu_collator(iculocstr, &result);
+
+			datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collicurules, &isnull);
+			if (!isnull)
+				icurules = TextDatumGetCString(datum);
+			else
+				icurules = NULL;
+
+			make_icu_collator(iculocstr, icurules, &result);
 		}
 
 		datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collversion,
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index a990c833c5..55a035c062 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -421,10 +421,19 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
 
 	if (dbform->datlocprovider == COLLPROVIDER_ICU)
 	{
+		char	   *icurules;
+
 		datum = SysCacheGetAttr(DATABASEOID, tup, Anum_pg_database_daticulocale, &isnull);
 		Assert(!isnull);
 		iculocale = TextDatumGetCString(datum);
-		make_icu_collator(iculocale, &default_locale);
+
+		datum = SysCacheGetAttr(DATABASEOID, tup, Anum_pg_database_daticurules, &isnull);
+		if (!isnull)
+			icurules = TextDatumGetCString(datum);
+		else
+			icurules = NULL;
+
+		make_icu_collator(iculocale, icurules, &default_locale);
 	}
 	else
 		iculocale = NULL;
diff --git a/src/include/catalog/pg_collation.h b/src/include/catalog/pg_collation.h
index 2190ccb5b8..ad2d767d65 100644
--- a/src/include/catalog/pg_collation.h
+++ b/src/include/catalog/pg_collation.h
@@ -43,6 +43,7 @@ CATALOG(pg_collation,3456,CollationRelationId)
 	text		collcollate BKI_DEFAULT(_null_);	/* LC_COLLATE setting */
 	text		collctype BKI_DEFAULT(_null_);	/* LC_CTYPE setting */
 	text		colliculocale BKI_DEFAULT(_null_);	/* ICU locale ID */
+	text		collicurules BKI_DEFAULT(_null_);	/* ICU collation rules */
 	text		collversion BKI_DEFAULT(_null_);	/* provider-dependent
 													 * version of collation
 													 * data */
@@ -91,6 +92,7 @@ extern Oid	CollationCreate(const char *collname, Oid collnamespace,
 							int32 collencoding,
 							const char *collcollate, const char *collctype,
 							const char *colliculocale,
+							const char *collicurules,
 							const char *collversion,
 							bool if_not_exists,
 							bool quiet);
diff --git a/src/include/catalog/pg_database.h b/src/include/catalog/pg_database.h
index 611c95656a..48bc285863 100644
--- a/src/include/catalog/pg_database.h
+++ b/src/include/catalog/pg_database.h
@@ -71,6 +71,9 @@ CATALOG(pg_database,1262,DatabaseRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID
 	/* ICU locale ID */
 	text		daticulocale;
 
+	/* ICU collation rules */
+	text		daticurules BKI_DEFAULT(_null_);
+
 	/* provider-dependent version of collation data */
 	text		datcollversion BKI_DEFAULT(_null_);
 
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index a875942123..989fe26cef 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -95,6 +95,7 @@ typedef struct pg_locale_struct *pg_locale_t;
 extern PGDLLIMPORT struct pg_locale_struct default_locale;
 
 extern void make_icu_collator(const char *iculocstr,
+							  const char *icurules,
 							  struct pg_locale_struct *resultp);
 
 extern pg_locale_t pg_newlocale_from_collation(Oid collid);
diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out
index d4c8c6de38..ce9ed3c8bf 100644
--- a/src/test/regress/expected/collate.icu.utf8.out
+++ b/src/test/regress/expected/collate.icu.utf8.out
@@ -1190,6 +1190,36 @@ SELECT 'Goldmann' < 'Götz' COLLATE "de-x-icu", 'Goldmann' > 'Götz' COLLATE tes
  t        | t
 (1 row)
 
+-- rules
+CREATE COLLATION testcoll_rules1 (provider = icu, locale = '', rules = '&a < g');
+CREATE TABLE test7 (a text);
+-- example from https://unicode-org.github.io/icu/userguide/collation/customization/#syntax
+INSERT INTO test7 VALUES ('Abernathy'), ('apple'), ('bird'), ('Boston'), ('Graham'), ('green');
+SELECT * FROM test7 ORDER BY a COLLATE "en-x-icu";
+     a     
+-----------
+ Abernathy
+ apple
+ bird
+ Boston
+ Graham
+ green
+(6 rows)
+
+SELECT * FROM test7 ORDER BY a COLLATE testcoll_rules1;
+     a     
+-----------
+ Abernathy
+ apple
+ green
+ bird
+ Boston
+ Graham
+(6 rows)
+
+DROP TABLE test7;
+CREATE COLLATION testcoll_rulesx (provider = icu, locale = '', rules = '!!wrong!!');
+ERROR:  could not open collator for locale "" with rules "!!wrong!!": U_INVALID_FORMAT_ERROR
 -- nondeterministic collations
 CREATE COLLATION ctest_det (provider = icu, locale = '', deterministic = true);
 CREATE COLLATION ctest_nondet (provider = icu, locale = '', deterministic = false);
diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql
index b0ddc7db44..aa95c1ec42 100644
--- a/src/test/regress/sql/collate.icu.utf8.sql
+++ b/src/test/regress/sql/collate.icu.utf8.sql
@@ -472,6 +472,19 @@ CREATE COLLATION testcoll_de_phonebook (provider = icu, locale = 'de@collation=p
 SELECT 'Goldmann' < 'Götz' COLLATE "de-x-icu", 'Goldmann' > 'Götz' COLLATE testcoll_de_phonebook;
 
 
+-- rules
+
+CREATE COLLATION testcoll_rules1 (provider = icu, locale = '', rules = '&a < g');
+CREATE TABLE test7 (a text);
+-- example from https://unicode-org.github.io/icu/userguide/collation/customization/#syntax
+INSERT INTO test7 VALUES ('Abernathy'), ('apple'), ('bird'), ('Boston'), ('Graham'), ('green');
+SELECT * FROM test7 ORDER BY a COLLATE "en-x-icu";
+SELECT * FROM test7 ORDER BY a COLLATE testcoll_rules1;
+DROP TABLE test7;
+
+CREATE COLLATION testcoll_rulesx (provider = icu, locale = '', rules = '!!wrong!!');
+
+
 -- nondeterministic collations
 
 CREATE COLLATION ctest_det (provider = icu, locale = '', deterministic = true);
-- 
2.38.1



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: dikkop seems unhappy because of openssl stuff (FreeBSD 14-BETA1)
@ 2023-09-29 23:57  Tom Lane <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Tom Lane @ 2023-09-29 23:57 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Tomas Vondra <[email protected]>; PostgreSQL Hackers <[email protected]>

Thomas Munro <[email protected]> writes:
> Does the image lack a /etc/localtime file/link, but perhaps one of you
> did something to create it?

Hah!  I thought it had to be some sort of locale effect, but I failed
to think of that as a contributor :-(.  My installation does have
/etc/localtime, and removing it duplicates Tomas' syndrome.

I also find that if I add "-gmt 1" to the clock invocation, it's happy
with or without /etc/localtime.  So I think we should modify the test
case to use that to reduce its environmental sensitivity.  Will
go make it so.

			regards, tom lane

^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: dikkop seems unhappy because of openssl stuff (FreeBSD 14-BETA1)
@ 2023-10-04 11:04  Tomas Vondra <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Tomas Vondra @ 2023-10-04 11:04 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Thomas Munro <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On 9/30/23 01:57, Tom Lane wrote:
> Thomas Munro <[email protected]> writes:
>> Does the image lack a /etc/localtime file/link, but perhaps one of you
>> did something to create it?
> 
> Hah!  I thought it had to be some sort of locale effect, but I failed
> to think of that as a contributor :-(.  My installation does have
> /etc/localtime, and removing it duplicates Tomas' syndrome.
> 
> I also find that if I add "-gmt 1" to the clock invocation, it's happy
> with or without /etc/localtime.  So I think we should modify the test
> case to use that to reduce its environmental sensitivity.  Will
> go make it so.
> 

FWIW I've defined the timezone (copying it into /etc/localtime), and
that seems to have resolved the issue (well, maybe it's the "-gmt 1"
tweak, not sure).

I wonder how come it worked with the earlier image - I don't recall
defining the timezone (AFAIK I only did the bare minimum to get it
working), but maybe I did.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

end of thread, other threads:[~2023-10-04 11:04 UTC | newest]

Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-12-07 05:18 Re: Strange OSX make check-world failure Samuel Cochran <[email protected]>
2018-12-07 06:26 ` Tom Lane <[email protected]>
2018-12-08 03:20   ` Samuel Cochran <[email protected]>
2020-03-19 01:54 [PATCH 5/7] fix Tomas Vondra <[email protected]>
2020-03-19 02:02 [PATCH 7/7] fix Tomas Vondra <[email protected]>
2020-03-19 15:55 [PATCH 4/8] fix Tomas Vondra <[email protected]>
2020-03-19 17:26 [PATCH 6/8] fix Tomas Vondra <[email protected]>
2020-03-19 17:48 [PATCH 8/8] fix Tomas Vondra <[email protected]>
2022-12-14 09:26 Allow tailoring of ICU locales with custom rules Peter Eisentraut <[email protected]>
2023-09-29 23:57 Re: dikkop seems unhappy because of openssl stuff (FreeBSD 14-BETA1) Tom Lane <[email protected]>
2023-10-04 11:04 ` Re: dikkop seems unhappy because of openssl stuff (FreeBSD 14-BETA1) Tomas Vondra <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox