Obsolete reference to _bt_tuplecompare() within tuplesort.c

Started by Peter Geogheganover 11 years ago5 messages
#1Peter Geoghegan
pg@heroku.com

I found a reference made obsolete by commit 9e85183b, which is from
way back in 2000.

comparetup_index_btree() says:

/*
* This is similar to _bt_tuplecompare(), but we have already done the
* index_getattr calls for the first column, and we need to keep track of
* whether any null fields are present. Also see the special treatment
* for equal keys at the end.
*/

I think that this comment should simply indicate that the routine is
similar to comparetup_heap(), except that it takes care of the special
tie-break stuff for B-Tree sorts, as well as enforcing uniqueness
during unique index builds. It should not reference _bt_compare() at
all (which is arguably the successor to _bt_tuplecompare()), since
_bt_compare() is concerned with a bunch of stuff highly specific to
the B-Tree implementation (e.g. having a hard-wired return value for
comparisons involving the first data item on an internal page).

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Heikki Linnakangas
hlinnakangas@vmware.com
In reply to: Peter Geoghegan (#1)
Re: Obsolete reference to _bt_tuplecompare() within tuplesort.c

On 10/10/2014 02:04 AM, Peter Geoghegan wrote:

I found a reference made obsolete by commit 9e85183b, which is from
way back in 2000.

comparetup_index_btree() says:

/*
* This is similar to _bt_tuplecompare(), but we have already done the
* index_getattr calls for the first column, and we need to keep track of
* whether any null fields are present. Also see the special treatment
* for equal keys at the end.
*/

I think that this comment should simply indicate that the routine is
similar to comparetup_heap(), except that it takes care of the special
tie-break stuff for B-Tree sorts, as well as enforcing uniqueness
during unique index builds. It should not reference _bt_compare() at
all (which is arguably the successor to _bt_tuplecompare()), since
_bt_compare() is concerned with a bunch of stuff highly specific to
the B-Tree implementation (e.g. having a hard-wired return value for
comparisons involving the first data item on an internal page).

Yeah. Want to write that into a patch?

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Peter Geoghegan
pg@heroku.com
In reply to: Heikki Linnakangas (#2)
1 attachment(s)
Re: Obsolete reference to _bt_tuplecompare() within tuplesort.c

On Thu, Oct 9, 2014 at 11:58 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:

Yeah. Want to write that into a patch?

Attached.

--
Peter Geoghegan

Attachments:

obsolete_comment.difftext/plain; charset=US-ASCII; name=obsolete_comment.diffDownload
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
new file mode 100644
index 8e57505..2d15da0
*** a/src/backend/utils/sort/tuplesort.c
--- b/src/backend/utils/sort/tuplesort.c
*************** comparetup_index_btree(const SortTuple *
*** 3176,3184 ****
  					   Tuplesortstate *state)
  {
  	/*
! 	 * This is similar to _bt_tuplecompare(), but we have already done the
! 	 * index_getattr calls for the first column, and we need to keep track of
! 	 * whether any null fields are present.  Also see the special treatment
  	 * for equal keys at the end.
  	 */
  	ScanKey		scanKey = state->indexScanKey;
--- 3176,3183 ----
  					   Tuplesortstate *state)
  {
  	/*
! 	 * This is similar to comparetup_heap(), but expects index tuples.  There
! 	 * is also special handling for enforcing uniqueness, and special treatment
  	 * for equal keys at the end.
  	 */
  	ScanKey		scanKey = state->indexScanKey;
#4Peter Geoghegan
pg@heroku.com
In reply to: Peter Geoghegan (#3)
Re: Obsolete reference to _bt_tuplecompare() within tuplesort.c

On Fri, Oct 10, 2014 at 12:33 AM, Peter Geoghegan <pg@heroku.com> wrote:

Attached

Have you looked at this?

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Heikki Linnakangas
hlinnakangas@vmware.com
In reply to: Peter Geoghegan (#4)
Re: Obsolete reference to _bt_tuplecompare() within tuplesort.c

On 10/19/2014 11:29 AM, Peter Geoghegan wrote:

On Fri, Oct 10, 2014 at 12:33 AM, Peter Geoghegan <pg@heroku.com> wrote:

Attached

Have you looked at this?

Committed now, thanks.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers