Hello.

In the discussion about cube's dimention limit [1], I found that
the error messages looks strange.

https://www.postgresql.org/message-id/f0e1a404-a495-4f38-b817-06355b537...@yandex-team.ru

> postgres=# create table y as  select cube(array(SELECT random() as a FROM 
> generate_series(1,1000))) from generate_series(1,1e3,1); 
> SELECT 1000
> postgres=# create index on y using gist(cube );
> ERROR:  index row size 8016 exceeds maximum 8152 for index "y_cube_idx"

This is apparently strange. This is because the message doesn't
count fill factor at the time. It is fixed by passing freespace
to gistSplit() and that allows gistfitpage() to consider
fillfactor as TODO comment within.

After the attached patch applied, the above messages becomes as
follows. (And index can be built being a bit sparse by fill
factor.)

> ERROR:  index row size 8016 exceeds maximum 7333 for index "y_cube_idx"

I'm not sure why 277807bd9e didn't do that completely so I may be
missing something. Is there any thoughts?


There's another issue that ununderstandable message is issued
when (the root) page cannot store two tuples. I'll send a fix for
that sooner if no one objects to check it separately.

> =# create table y as select cube(array(SELECT random() as a FROM 
> generate_series(1,900))) from generate_series(1,1e3,1);
> =# create index on y using gist (cube);
> ERROR:  failed to add item to index page in "y_cube_idx"

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 8a42effdf7..7773a55552 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -293,7 +293,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate,
 				memmove(itvec + pos, itvec + pos + 1, sizeof(IndexTuple) * (tlen - pos));
 		}
 		itvec = gistjoinvector(itvec, &tlen, itup, ntup);
-		dist = gistSplit(rel, page, itvec, tlen, giststate);
+		dist = gistSplit(rel, page, itvec, tlen, freespace, giststate);
 
 		/*
 		 * Check that split didn't produce too many pages.
@@ -1352,6 +1352,7 @@ gistSplit(Relation r,
 		  Page page,
 		  IndexTuple *itup,		/* contains compressed entry */
 		  int len,
+		  int freespace,
 		  GISTSTATE *giststate)
 {
 	IndexTuple *lvectup,
@@ -1374,7 +1375,7 @@ gistSplit(Relation r,
 		ereport(ERROR,
 				(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 				 errmsg("index row size %zu exceeds maximum %zu for index \"%s\"",
-						IndexTupleSize(itup[0]), GiSTPageSize,
+						IndexTupleSize(itup[0]), GiSTPageSize - freespace,
 						RelationGetRelationName(r))));
 
 	memset(v.spl_lisnull, true, sizeof(bool) * giststate->tupdesc->natts);
@@ -1392,9 +1393,10 @@ gistSplit(Relation r,
 		rvectup[i] = itup[v.splitVector.spl_right[i] - 1];
 
 	/* finalize splitting (may need another split) */
-	if (!gistfitpage(rvectup, v.splitVector.spl_nright))
+	if (!gistfitpage(rvectup, v.splitVector.spl_nright, freespace))
 	{
-		res = gistSplit(r, page, rvectup, v.splitVector.spl_nright, giststate);
+		res = gistSplit(r, page, rvectup, v.splitVector.spl_nright, freespace,
+						giststate);
 	}
 	else
 	{
@@ -1404,12 +1406,13 @@ gistSplit(Relation r,
 		res->itup = gistFormTuple(giststate, r, v.spl_rattr, v.spl_risnull, false);
 	}
 
-	if (!gistfitpage(lvectup, v.splitVector.spl_nleft))
+	if (!gistfitpage(lvectup, v.splitVector.spl_nleft, freespace))
 	{
 		SplitedPageLayout *resptr,
 				   *subres;
 
-		resptr = subres = gistSplit(r, page, lvectup, v.splitVector.spl_nleft, giststate);
+		resptr = subres = gistSplit(r, page, lvectup, v.splitVector.spl_nleft,
+									freespace, giststate);
 
 		/* install on list's tail */
 		while (resptr->next)
diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
index dddfe0ae2c..09c7f621bc 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -74,7 +74,7 @@ gistnospace(Page page, IndexTuple *itvec, int len, OffsetNumber todelete, Size f
 }
 
 bool
-gistfitpage(IndexTuple *itvec, int len)
+gistfitpage(IndexTuple *itvec, int len, int freespace)
 {
 	int			i;
 	Size		size = 0;
@@ -82,8 +82,7 @@ gistfitpage(IndexTuple *itvec, int len)
 	for (i = 0; i < len; i++)
 		size += IndexTupleSize(itvec[i]) + sizeof(ItemIdData);
 
-	/* TODO: Consider fillfactor */
-	return (size <= GiSTPageSize);
+	return (size <= GiSTPageSize - freespace);
 }
 
 /*
diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h
index 36ed7244ba..cac3d0b8e5 100644
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -407,7 +407,7 @@ extern bool gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate,
 				bool markleftchild);
 
 extern SplitedPageLayout *gistSplit(Relation r, Page page, IndexTuple *itup,
-		  int len, GISTSTATE *giststate);
+				int len, int freespace, GISTSTATE *giststate);
 
 extern XLogRecPtr gistXLogUpdate(Buffer buffer,
 			   OffsetNumber *todelete, int ntodelete,
@@ -439,7 +439,7 @@ extern bytea *gistoptions(Datum reloptions, bool validate);
 extern bool gistproperty(Oid index_oid, int attno,
 			 IndexAMProperty prop, const char *propname,
 			 bool *res, bool *isnull);
-extern bool gistfitpage(IndexTuple *itvec, int len);
+extern bool gistfitpage(IndexTuple *itvec, int len, int freespace);
 extern bool gistnospace(Page page, IndexTuple *itvec, int len, OffsetNumber todelete, Size freespace);
 extern void gistcheckpage(Relation rel, Buffer buf);
 extern Buffer gistNewBuffer(Relation r);

Reply via email to