Ticket #1140 (closed defect: fixed)

Opened 22 months ago

Last modified 22 months ago

loader/PointZ test failing on my windows 9.0/9.1

Reported by: robe Owned by: strk
Priority: medium Milestone: PostGIS 2.0.0
Component: postgis Version: trunk
Keywords: Cc:

Description

I haven't tried my 8.4 yet, I presume it's strk's last r7656 change?

It seems now my results are sorted differently from expected: The PointZ.select.expected has:

POINT(0 1 2 3)
POINT(9 -1 -20 -123)
POINT(9 -1 -2 -3)

But I'm getting

POINT(0 1 2 3)
POINT(9 -1 -2 -3)
POINT(9 -1 -20 -123)

I guess 0 sorts after space on windows.

What do you get strk:

e.g.

SELECT *
FROM (values ('01'),(' 03')) As f
ORDER By 1;

on my box gives:

 03
01

Attachments

run_test_collate.diff Download (0.6 KB) - added by ads 22 months ago.
add collate and template to createdb
regress_collate.patch Download (1.5 KB) - added by strk 22 months ago.
comprehensive patch for trunk, apply with patch -p1

Change History

Changed 22 months ago by robe

  • owner changed from pramsey to strk

update this also fails on my 8.4.8 windows vc++ build (against mingw compiled PostGIS)

Changed 22 months ago by ads

I get the exact opposite order:

ads=# SELECT * FROM (values ('01'),(' 03')) AS f ORDER BY 1;
 column1 
---------
 01
  03
(2 Zeilen)

What is your Collate setting?
See:  http://www.depesz.com/index.php/2011/06/30/why-is-depesz-between-de-luca-and-de-vil/

Changed 22 months ago by strk

We're supposed to be running tests with the C locale, are we?

Changed 22 months ago by ads

At least that's what run_test is doing:

# Set the locale to "C" so error messages match
# Save original locale to set back
ORIG_LC_ALL=$LC_ALL
ORIG_LANG=$LANG
export LC_ALL=C
export LANG=C

if [ -n "$USE_VERSION" ]; then
	if [ "$USE_VERSION" -gt 74 ]; then
		PGOPTIONS="${PGOPTIONS} -c lc_messages=C"
		export PGOPTIONS
	fi
fi

So why is this breaking the regression test?

Changed 22 months ago by robe

I fiddled with the locale posix/c/us something or other -- doesn't change my sorting

Changed 22 months ago by robe

I also tried on a different windows box running 8.4 - created a new db with local c

CREATE DATABASE test
  WITH OWNER = postgres
       ENCODING = 'UTF8'
       TABLESPACE = pg_default
       LC_COLLATE = 'C'
       LC_CTYPE = 'C'
       CONNECTION LIMIT = -1;


Ran my little test:

SELECT * FROM (values ('01'),(' 03')) AS f ORDER BY 1;

Output:

 column1
---------
  03
 01

Changed 22 months ago by robe

hmm I wonder if I can't have UTF-8 database and C-locale. In the article you pointed to it says you can't do this:

ERROR: collation "c" for encoding "UTF8" does not exist

Indeed 9.1 has other issues

SELECT * FROM (values ('01'),(' 03')) AS f(a) ORDER BY 1 collate somethingorother;

gives
ERROR:  collations are not supported by type integer

Is illegal and it reads 1 as if it were an integer rather than a column.

I don't have any collations installed in my 9.1 so I created one which I would think would be illegal for UTF-8, but it let me do it:

CREATE COLLATION ucs_basic
  (LC_COLLATE='C', LC_CTYPE='C');
SELECT * FROM (values ('01'),(' 03')) AS f(a) ORDER BY a collate ucs_basic;

still didn't change the answer

Output:

 a
---------
  03
 01

Changed 22 months ago by ads

Well, after further testing it looks like your sorting is right and my previous posting is misleading - and the test case seems to have a minor problem:
The only LC_* setting is LC_MESSAGES on top of the file.
LC_COLLATE is never set and therefore depends on whatever the database is using by default.

In addition, I can't create a database with LC_COLLATE='C' - unless I use template0 as template.

So the tests are only run with C locale, if you have set OS and database locale this way.

Changed 22 months ago by ads

Hi, can you please try and test the attached patch with postgis-2.0.0SVN?
It works with PostgreSQL 9.0.x for me, but of course breaks my patch from #1135.

Changed 22 months ago by ads

add collate and template to createdb

Changed 22 months ago by strk

I confirm it breaks the PointZ test:

--- loader/PointZ-w.select.expected     2011-07-20 21:51:54.000000000 +0200
+++ /tmp/pgis_reg_22295/test_3_out      2011-07-22 23:59:46.000000000 +0200
@@ -1,3 +1,3 @@
 POINT(0 1 2 3)
-POINT(9 -1 -20 -123)
 POINT(9 -1 -2 -3)
+POINT(9 -1 -20 -123)

Should we then apply and expect the new order ?

Changed 22 months ago by ads

If everyone is fine with the new createdb options (please test in-depth) in run_test_collate.diff, I can provide a new patch for #1135.
Therefore it probably makes sense to revert #1135 first, run a number of tests on different platforms with the new patch, and then apply a new patch for #1135.
Opinions?

Changed 22 months ago by strk

I think the simplest thing for everyone is to apply a single patch against current HEAD and see if it gives _no_ failures. When successful, we can do the same for 1.5 branch.

So I suggest you provide another all-inclusive patch containing both the run_test modification _and_ the .expected files modification.

Changed 22 months ago by strk

comprehensive patch for trunk, apply with patch -p1

Changed 22 months ago by strk

I attached the full patch for trunk. Successfully passes make check on my postgresql 8.4.8 on 64bit GNU/Linux

Changed 22 months ago by ads

Well, I wanted to solve the collate problem first, before I apply for a new patch ;-)

Changed 22 months ago by robe

Fix works for my 8.4, 9.0, and 9.1beta3 builds. Andrea I assume you are all set too? strk you want to go ahead and commit and close this out and the other out or you want me to?

Thanks, Regina

Changed 22 months ago by robe

meant Andreas sorry for the typo.

Changed 22 months ago by strk

@robe: feel free to go ahead if 'ads' confirms it works for him as well.

Changed 22 months ago by strk

  • status changed from new to closed
  • resolution set to fixed

Actually, I committed it as r7665 and we can tweak it further if needed.

Changed 22 months ago by ads

Since it worked for me with PostgreSQL, I'm fine. Will backport the patch to Greenplum - but that's my part.

Note: See TracTickets for help on using tickets.