Opened 6 years ago

Closed 6 years ago

#7218 closed defect (fixed)

ogr2ogr -addfields issue

Reported by: plesk Owned by: warmerdam
Priority: normal Milestone:
Component: default Version: 2.2.1
Severity: normal Keywords:
Cc:

Description

Using the -addfields flag fails in some cases when the new field is set to NOT NULL. Experienced in POSTGRES Driver, unsure of others.

When altering the destination table, a column is sometimes created using 'ALTER... NOT NULL' (see below for error). Altering a table and setting not null on a new column will always fail when there is existing data in the table. The not null constraint should either be left out, defaulted or applied after data is migrated.

ERROR 1: ALTER TABLE "datavic"."cultural_sensitivity" ADD COLUMN "gml_id" VARCHAR NOT NULL
ERROR:  column "gml_id" contains null values

Change History (6)

comment:1 by Jukka Rahkonen, 6 years ago

You play with WFS which is sending you GML and in GML fid is a compulsory field. In general case I think it is right to consider fid as NOT NULL as for example ogrinfo does

ogrinfo WFS:"http://services.land.vic.gov.au/catalogue/publicproxy/guest/dv_geoserver/datavic/wfs?version=1.0.0" datavic:CULTURE_SENSITIVITY_PUBLIC -so
Geometry Column = SHAPE
gml_id: String (0.0) NOT NULL
SENSITIVITY: String (0.0)
OBJECTID: Integer (0.0)

I understand what you want to do but I am not at all sure if this is a bug. You should rather analyze your use case and re-think how to make the updates to work. Perhaps you need to use interim tables and do the rest with SQL in the database. The SQLite dialect also gives you better possibilities to modify the dataset with SQL.

comment:2 by plesk, 6 years ago

Fair enough, I understand why the fid is required in the source but it is not always required in the destination. The addfields option was a solution to painless source schema changes, but looks like it won't work when excluding the fid. I think a rethink on our end is required.

comment:3 by Even Rouault, 6 years ago

Try adding -forceNullable

comment:4 by plesk, 6 years ago

-forceNullable works, but I feel the addfields function itself should handle such cases. I thought the purpose of -addfields was to make it easier to handle schema changes. You want it to retain the field type definitions (inc not null) but not to fail on something like a not null constraint.

This problem also occurs when the field being added is not the pk. In the following example a not null column is added to the db and causes the problem.

CREATE TABLE public.temp
(
    id integer NOT NULL,
    job text
)
ogr2ogr "PG:###" "PG:###" public.temp -nln public.temp_copy -f postgresql -append -addfields
 > success

INSERT INTO public.temp (id,job) values (1,'test');
ALTER TABLE public.temp ADD COLUMN test TEXT NOT NULL DEFAULT 'test';

ogr2ogr "PG:###" "PG:###" public.temp -nln public.temp_copy -f postgresql -append -addfields

ERROR 1: ERROR:  column "test" contains null values

ERROR 1: ALTER TABLE "temp1" ADD COLUMN "test" VARCHAR NOT NULL
ERROR:  column "test" contains null values

ERROR 1: ERROR:  current transaction is aborted, commands ignored until end of t
ransaction block

ERROR 1: INSERT command for new feature failed.
ERROR:  current transaction is aborted, commands ignored until end of transactio
n block

Command: INSERT INTO "temp1" ("job") VALUES ('test1') RETURNING "ogc_fid"
ERROR 1: Unable to write feature 1 from layer temp.
ERROR 1: Terminating translation prematurely after failed
translation of layer temp (use -skipfailures to skip errors)

comment:5 by Jukka Rahkonen, 6 years ago

It might be reasonable to use -forceNullable by default with -addfields. However, user who plans to collect datasets with different schemas should actually know that beforehand and use -forceNullable already when the new layer is created. Think about a situation when you just add data in different order and the first dataset has that not nullable "test" field. Insert is OK but you can't add the other dataset that does not contain the "test" field. But it would not be good to make -forceNullable as a default when the layer is created.

Perhaps this should be mentioned in the ogr2ogr manual page, in -addfields or in -forceNullable?

comment:6 by Even Rouault, 6 years ago

Resolution: fixed
Status: newclosed

In 41371:

ogr2ogr doc: mention -forceNullable in -addfields doc (fixes #7218) [ci skip]

Note: See TracTickets for help on using tickets.