shp2pgsql use wrong schema name

[aperi2007]

Hi,

try-ing to load a shapefile on postgis 2.0.0 trunk using shp2pgsql.exe I notice that it produce a wrong sql file if the destination table has a schema. Infact if I call using: shp2pgsql.exe" -s 3003 -d -g geom -D -i -I -W "UTF-8" comuni.shp public.comuni > comuni.sql

asking to put the data in a table of a public schema I see the sql file a wrong schema name having a dot not necessary: 'public.' I add a portion of the sql generate to see the wrong schema name used.

This error is available even in the debian compiled from source.

---

SET CLIENT_ENCODING TO UTF8; SET STANDARD_CONFORMING_STRINGS TO ON; SELECT DropGeometryColumn?('public.','comuni','geom'); DROP TABLE "public."."comuni"; BEGIN; CREATE TABLE "public."."comuni" (gid serial PRIMARY KEY, "area" float8, "perimeter" float8, "am_com_" float8, "am_com_id" float8, "codcom" varchar(6), "nome" varchar(100), "codprov" varchar(3)); SELECT AddGeometryColumn?('public.','comuni','geom','3003','MULTIPOLYGON',2); COPY "public."."comuni" ("area","perimeter","am_com_","am_com_id","codcom","nome","codprov",geom) FROM stdin;

[mcayland]

I can't seem to reproduce this on SVN trunk using some of my test shapefiles - perhaps it is something specific to that file? Is it publically available?

[aperi2007]

It happened with more shapefiles. I try with three distinct shapefile.

The problem is in the use of schema. If you try using

shp2pgsql.exe" -s 3003 -d -g geom -D -i -I -W "UTF-8" goofy.shp table_destination > goofy.sql

You will have a regular and perfectly usable file sql.

But if you add the schema-name to the table_destination (public or other) :

for example:

shp2pgsql.exe" -s 3003 -d -g geom -D -i -I -W "UTF-8" goofy.shp public.table_destination > file.sql

You will have a not usable file sql. Infact see-ing it you can see it have a double dot in every situation where there is a citation of schema-name.

for examples.

... DROP TABLE "public."."table_destination" ...

and so on.

Is as the shp2pgsql when detect the presence of a "schema name" in the command line, it want add a dot to the schema name, causing the presence of a double dot.

I verified this happened only with the postigis 2.0 version of shp2pgsql.

Actually to bypass this problem I must to omit the presence of schema-name in the table_destination and so load all in a default schema.

Can you confirm this situation, thx ?

[mcayland]

On my Debian amd64 box I get the following with SVN trunk (r6549):

./shp2pgsql -s 3003 -d -g geom -D -i -I -W "UTF-8" world_borders.shp public.foo | less

SET CLIENT_ENCODING TO UTF8; SET STANDARD_CONFORMING_STRINGS TO ON; SELECT DropGeometryColumn?('public','foo','geom'); DROP TABLE "public"."foo"; BEGIN; CREATE TABLE "public"."foo" (gid serial PRIMARY KEY, "cat" float8, "fips_cntry" varchar(80), "cntry_name" varchar(80), "area" float8, "pop_cntry" float8); SELECT AddGeometryColumn?('public','foo','geom','3003','MULTIPOLYGON',2); COPY "public"."foo" ("cat","fips_cntry","cntry_name","area","pop_cntry",geom) FROM stdin; 1 AA Aruba 193.00 71218.00 0106000020BB0B00000100000001030000000100000013000000ED9C6681767851C0FC1D8A027DD22840168733BF9A7C51C0DF8C9AAF92DF284083A5BA80978351C0FA264D83A211294002F4FBFECD8351C008E6E8F17B132940EF5701BEDB8351C0F71F990E9D162940ED6305BF0D8451C0FD1186014B3E29406C21C841098451C0CCCF0D4DD941294018B14F00C58351C02B6C06B820432940ED9925016A8351C0763579CA6A4229402C11A8FE418251C0AF5A99F04B3D2940C05E61C1FD8151C004FEF0F3DF3B2940D9092FC1A97B51C083FA96395D0E2940C53A55BE677951C0D6AC33BE2FF628409A232BBF0C7951C005F86EF3C6F128406CEBA7FFAC7851C0D4B837BF61EA284045BC75FEED7751C0A27C410B09D82840C158DFC0E47751C0F4311F10E8D4284056647440127851C0D0D03FC1C5D22840ED9C6681767851C0FC1D8A027DD22840

So it looks okay to me here? The world_borders shapefile I used for testing was obtained from  http://mappinghacks.com/data/.

[aperi2007]

Hi,

I have to do a mistake. I tested the shp2pgsql try-ing to load on both : a windows postgres and a linux postgres.

But always using the shp2pgsql.exe of my window machine !

Uncorrectly I was thinking the bug was even in the linux version of shp2pgsql.

So the bug is not on the shp2pgsql of Linux version but only in the shp2pgsql.exe of the windows version.

Thx for the correction for Linux version.

I test again and the bug on windows version is confirmed on my machine. I use the 30 december version of postgis 2.0 per win32, with a postgres 9.0.1 updated to 9.0.2.

[mcayland]

Hmmmm the only thing I can think of is that this is a bug in the PostGIS or MingW versions of getopt() - Paul/Regina??

[robe]

Mark,

I tried with a table using my December 30th build I had posted which is the one Andrea is using, and I get the same problem with any table.

It stuffs in an extra ..

when I run this:

shp2pgsql -s 4269 -d -g geom -D -i -I -W "UTF-8" place public.place > places.sql

The output of my file looks like:

SELECT DropGeometryColumn('public.','place','geom');
DROP TABLE "public."."place";

I tried with different schemas and quoting the schema and it does the same thing - its not parsing the schema name out right.

Mark -- I presume its more than just a MingW problem since Andrea says it happens on his Debian install as well. I don't think its getopt either.

I think maybe the period check or something is wrong in shp2pgsql-cli.c (somewhere around line 185-190) and maybe its specific to 32/64 bit maybe?

[aperi2007]

Mark -- I presume its more than just a MingW problem since Andrea says it happens on >his Debian install as well. I don't think its getopt either.

Sorry, I do a wrong test for linux version.

In the Debian instance all work correctly.

The bug is only in windows version.

[robe]

Those lines I mentioned haven't changed since 1.5. I tested my 1.5.2 build and its okay.

I suspect something in line 59-63 (these lines were added since)

#ifdef USE_NLS
	setlocale (LC_ALL, "");
	bindtextdomain (PACKAGE, LOCALEDIR);
	textdomain (PACKAGE);
#endif

That maybe somehow changed the length of things so the length ptr position being calculated in on line 185:

ptr = strchr(argv[pgis_optind], '.');

Is now off by one character

or I'm using the wrong version of something.

[robe]

BTW if I rebuild 1.5 branch. It too is okay so definitely something introduced in 2.0.

[mcayland]

Yuck. Can you do an SVN bisect to locate the culprit commit?

For Paul:

Note that the USE_NLS flag has been implemented correctly - it should not be passed in as a -D option into the build, but should be part of postgis_config.h so it can be disabled there (think of MSVC builds, compilers with different command arguments for example).

postgis_config.h.in is autogenerated by one of the autotools utilities, although I seem to remember I had to remove a couple of unused entries in the past to prevent name clashes.

[robe]

You think its related to these warnings I've been getting since Paul put in all that new NLS stuff?

" -Ic:/projects/pg/pg90/include/POSTGR~1/server/port/win32  -c -o safileio.o safileio.c
safileio.c:75: warning: ignoring #pragma comment 
safileio.c:213: warning: no previous prototype for 'Utf8ToWideChar'
safileio.c:241: warning: no previous prototype for 'SAUtf8WFOpen'
safileio.c:260: warning: no previous prototype for 'SAUtf8WRemove'

gcc -g -O2  -DDLL_EXPORT -DPIC  -Wall -Wmissing-prototypes  -DUSE_NLS -DLOCALEDIR=\"c:/projects/pg/pg90/share/locale\"   shpopen.o dbfopen.o getopt.o shp2pgsql-core.o shpcommon.o shp2pgsql-cli.o safileio.o ../liblwgeom/liblwgeom.a -o shp2pgsql.exe -lintl -liconv -lm 

I've been ignoring them since they didn't seem to cause any problems with regress etc.

[robe]

somewhere between r6239-r6357 is the problem.

r6238 is the last one that doesn't have this problem and then from r6239-r6356 I can't compile those.

[robe]

Oops typo I meant r6239-r6245 (I have to double-check on the end revision) and verify I can't compile 6244.

[mcayland]

Right - that definitely points the finger at the i18n stuff. Can you try the following on SVN trunk:

make clean ./configure

Then alter the generated loader/Makefile and comment out the NLS flags section like this:

# GetText? includes and libraries GETTEXT_CFLAGS = GETTEXT_LDFLAGS = #nls_build = no #ifdef nls_build # localedir = $(shell $(PG_CONFIG) --localedir) # CFLAGS += -DUSE_NLS # CFLAGS += -DLOCALEDIR=\"$(localedir)\" # LANGUAGES = fr #endif

If you then do a build, does it work after that?

[robe]

Yes that works.

For what it's worth, my LDFLAGS had a -intl (it wasn't blank like what you have). keeping that in like below also seems to work okay

 GETTEXT_CFLAGS =
 GETTEXT_LDFLAGS = -lintl
 #nls_build = no
 #ifdef nls_build
 #       localedir = $(shell $(PG_CONFIG) --localedir)
 #       CFLAGS += -DUSE_NLS
 #       CFLAGS += -DLOCALEDIR=\"$(localedir)\"
 #       LANGUAGES = fr
 #endif

Not to be discrinatory but hmm why do we have LANGUAGES french and no english or spanish or others? What's so special about French? Aside from those French Canadians.

[pramsey]

I was testing with French. We don't actually have any languages because we have no translations yet. I'm not going to push people on it though until we're certain the UI is settled and not going to change at all.

[robe]

Paul or Mark - are one of you going to revise the configure script or am I going to have to hack my windows build script all the time to get this to work?

[robe]

Andrea,

I rebuilt using Mark's hack for now - until Paul or Mark commit. It seems to not have the same issue, but who knows might have others.

Please give them it a try:

 http://www.postgis.org/download/windows/experimental.php

[aperi2007]

Hi Regina,

I test your Rebuild (10jan2011). The new sh2pgsql.exe work correctly for me.

Regards,

[jadams]

I just added a regression test (loader/PointWithSchema) that tries to load a shapefile using a schema-qualified name. Can you verify this test fails on windows?

I am suspicious that this line is wrong:

shp2pgsql-cli.c 248: snprintf(config->schema, ptr - argv[pgis_optind] + 1, "%s", argv[pgis_optind]);

(note line numbers have changed since earlier comments, this is in the section

/* Determine the table and schema names from the next argument */)

It's doing some pointer math to decide how many characters to copy, which makes me suspicious in the UTF8 / encoding / wide character crazy world we live in nowadays.

I'd suggest trying this instead:

snprintf(config->schema, strlen(argv[pgis_optind]) - strlen(ptr) + 1, "%s", argv[pgis_optind]);

Unfortunately I don't have a windows build so I can't test it.

[robe]

Jeff,

I'll try to unhack my hadcked windows. I had patched it with Mark's hack as a work around. I'll need to take that out to test. Are your changes for this already committed?

[jadams]

I committed the regression test (which passes on linux). I did not commit the code change for that one line because I don't want to change it if it doesn't fix the problem.

[robe]

Jeff,

I'm having trouble running the new regress tests on my windows install -- its giving these complaints:

loader/TSTPolygongrep: invalid option -- m Usage: grep [OPTION]... PATTERN [FILE]... Try `grep --help' for more information.

I'm also now getting failure on tickets regress. I'm investigating that one.

I vaguely recall we had issues with grep in mingw before. I think the solution might have been to use the long name of the option rathr than the short-name.

FWIW here is what my mingw grep output returns:

Usage: grep [OPTION]... PATTERN [FILE] ...
Search for PATTERN in each FILE or standard input.
Example: grep -i 'hello world' menu.h main.c

Regexp selection and interpretation:
  -E, --extended-regexp     PATTERN is an extended regular expression
  -F, --fixed-strings       PATTERN is a set of newline-separated strings
  -G, --basic-regexp        PATTERN is a basic regular expression
  -e, --regexp=PATTERN      use PATTERN as a regular expression
  -f, --file=FILE           obtain PATTERN from FILE
  -i, --ignore-case         ignore case distinctions
  -w, --word-regexp         force PATTERN to match only whole words
  -x, --line-regexp         force PATTERN to match only whole lines
  -z, --null-data           a data line ends in 0 byte, not newline

Miscellaneous:
  -s, --no-messages         suppress error messages
  -v, --invert-match        select non-matching lines
  -V, --version             print version information and exit
      --help                display this help and exit
      --mmap                use memory-mapped input if possible

Output control:
  -b, --byte-offset         print the byte offset with output lines
  -n, --line-number         print line number with output lines
  -H, --with-filename       print the filename for each match
  -h, --no-filename         suppress the prefixing filename on output
  -q, --quiet, --silent     suppress all normal output
      --binary-files=TYPE   assume that binary files are TYPE
                            TYPE is 'binary', 'text', or 'without-match'.
  -a, --text                equivalent to --binary-files=text
  -I                        equivalent to --binary-files=without-match
  -d, --directories=ACTION  how to handle directories
                            ACTION is 'read', 'recurse', or 'skip'.
  -r, --recursive           equivalent to --directories=recurse.
  -L, --files-without-match only print FILE names containing no match
  -l, --files-with-matches  only print FILE names containing matches
  -c, --count               only print a count of matching lines per FILE
  -Z, --null                print 0 byte after FILE name

Context control:
  -B, --before-context=NUM  print NUM lines of leading context
  -A, --after-context=NUM   print NUM lines of trailing context
  -C, --context[=NUM]       print NUM (default 2) lines of output context
                            unless overridden by -A or -B
  -NUM                      same as --context=NUM
  -U, --binary              do not strip CR characters at EOL (MSDOS)
  -u, --unix-byte-offsets   report offsets as if CRs were not there (MSDOS)

`egrep' means `grep -E'.  `fgrep' means `grep -F'.
With no FILE, or when FILE is -, read standard input.  If less than
two FILEs given, assume -h.  Exit status is 0 if match, 1 if no match,
and 2 if trouble.

Report bugs to <bug-gnu-utils@gnu.org>.

[jadams]

I removed the -m1 parameter. It was to ensure grep only read one line from the .opts command-line-options-for-the-test file, but you shouldn't ever have more than one non-comment line anyway so if you put two and it breaks, no big deal. There didn't seem to be a "max-lines" option in your MinGW grep.

[robe]

Jeff,

Okay the regress tests are now working and failed as expected at your schema check test.

Unfortunately the patch you have above didn't seem to make it work.

[robe]

Actually hold on I'm looking at the error. It might just be the expected file isn't formatted right since it looks like its inserting.

Let me try putting in the svn prop to change to line feeds.

[robe]

Well its still failing and so is the Latin1 test. I'll put that not on the other. I'm not sure if they are true failures quite yet.

[jadams]

Does the actual output look correct? If the error message says the diff that failed was "blah\blah\test_19_diff" then "blah\blah\test_19_out" will have the output.

I notice one of the diff commands in run_test uses "-u" and one does not, -u is something to do with ignoring CRs, so maybe we need -u in all places?

[robe]

This one has just loader.err as output. It has no associated diff I can see so I presume that means it is not getting to the select?

When I look at loader file - I see the insert is actually for the Latin one not this one so perhaps that means it didn't work. Unfortuantely I had dropped my regress database. I'll inspect that in a minute.

[robe]

okay looking at that didn't help. I'm going to put back in mark's hack to see if it passes. Then at least I'll know it's nothing wrong with the test itself.

[robe]

Okay the test is fine, it really is failing. If I put back in Mark's hack the test passes. So I guess since it fails on the load, it never gets to the part of doing an expected output. Though the load.err file gets overwritten with the Latin issue so is useless for diagnosing.

[jadams]

If you edit regress/Makefile and move this test after all the other loader/foo tests, its output won't get overwritten any more. Let me know if that gives you anything more to go on.

[robe]

Okay that helped -- this is the error (after I put in your fix)

SET
SET
BEGIN
ERROR:  invalid byte sequence for encoding "UTF8": 0x8b

[robe]

I'm curious though -- the make file change that Mark had me patch. I noticed on his example he had

nls_build = no

But mine of course registers as nls_build = yes

What does yours register as? Is it an issue for any system that actually goes into that loop or just a MingW flaw.

[jadams]

I also had nls_build = no, but I just tried changing it to yes and that didn't make a difference. I cannot reproduce the problem on linux, I don't know if it is specifically MingW or if there is some situation I'm just not replicating.

I may have to look more at this once I'm back at the office and have a windows machine I can develop on. I need to debug it more actively than is possible via comments on a ticket ;-). Thanks for your help so far!

[robe]

this is a windows only issue and one I have a work around in place for. The only quick fix would be to take out the offending lines which would make menus/messages in different languages impossible. I don't see this as being fixed for 2.0.0 and not a major issue.

[mcayland]

Note as of r8807 you should now be able to specify --with-gettext=no to configure in order to explicitly disable NLS at build time (this seems much more preferable as a temporary solution rather than you having to keep patching the source tree).

[robe]

thanks Mark for doing this. I'll give it a try.

[robe]

works great and I don't get a stupid alert about my gettext anymore either that I have to click to continue.

[mcayland]

Great! I can only apologise that it took me so long to commit the workaround :/