[postgis-devel] shp2pgsql patch

strk at refractions.net strk at refractions.net
Mon Apr 4 05:45:01 PDT 2005


Oh.. I've found the transaction issue.
Since I remember the shp2pgsql code splitted the load into 
multiple transactions. This actually makes it hard
to use the transaction as an integrity check as only the
block of records in the transaction containing a failur will
be skipped while the END; BEGIN; statements will start a fresh
one.

Maybe Jeff knows why it is like that, or Paul (but he's out of
office for a couple of weeks).

--strk;

On Mon, Apr 04, 2005 at 12:01:06PM +0200, strk at refractions.net wrote:
> On Fri, Apr 01, 2005 at 06:34:37PM +0200, Markus Schaber wrote:
> > Hi, Strk,
> > 
> > Refactoring the main loop into its own function makes the patch
> > noticeably smaller.
> > 
> > I now also moved the deletion of old tables (-d mode) into the
> > BEGIN;END; block. This protects the user from losing his old table when
> > the insertion of new data fails.
> > 
> > Or are there any good reasons to exclude this from transaction?
> 
> It was intended as a "make sure I'm building a new table" option.
> So that a failure in DROP would not fail the operation.
> But it is probably better to just wrap the whole file in a transaction
> and let the user try -c first and -d if it fails due to presence
> of another table...
> 
> I'd include your patch as it is, w/out requiring RC7.
> Does it sound azardous ?
> 
> --strk;
> 
> > 
> > Markus
> 
> > Index: README.shp2pgsql
> > ===================================================================
> > RCS file: /home/cvs/postgis/postgis/loader/README.shp2pgsql,v
> > retrieving revision 1.3
> > diff -u -r1.3 README.shp2pgsql
> > --- README.shp2pgsql	4 May 2002 22:44:04 -0000	1.3
> > +++ README.shp2pgsql	1 Apr 2005 16:30:44 -0000
> > @@ -38,19 +38,23 @@
> >  
> >  The options are as follows:
> >  
> > -(-a || -c || -d) these options are mutually exclusive.
> > +(-a || -c || -d || -p) these options are mutually exclusive.
> >  
> >    -a    Append mode. Do not delete the target table or try to create
> >          a new table, simple insert the data into the existing table.
> >          A table will have to exist for this to work, it is usually
> > -        used after a create mode as been run once.(mutually exclusive
> > -	with -c and -d)
> > +        used after a create mode as been run once or after -p. (mutually
> > +        exclusive with -c, -d and -p)
> >    -c    Create mode. This is the default mode is no other is specified.
> >  	Create a new table and upload the data into that table.
> > -	(mutually exclusive with -a and -d)
> > +	(mutually exclusive with -a, -d and -p)
> >    -d    Delete mode. Delete the database table named <tablename>, then
> >  	create a new one with that name before uploading the data into
> > -	the new empty database table.(mutually exclusive with -a and -c)
> > +	the new empty database table. (mutually exclusive with -a, -c 
> > +        and -p)
> > +  -p    Prepare mode. Read the table schema from the shape file and 
> > +        create the new table, but do not insert any data. (mutually
> > +        exclusive with -a, -c and -d)
> >  
> >    -D Dump. When inserting the data into the table use 'dump' format.
> >  	Dump format is used by PostgreSQL for large data dumps and 
> > Index: shp2pgsql.c
> > ===================================================================
> > RCS file: /home/cvs/postgis/postgis/loader/shp2pgsql.c,v
> > retrieving revision 1.83
> > diff -u -r1.83 shp2pgsql.c
> > --- shp2pgsql.c	15 Mar 2005 12:24:40 -0000	1.83
> > +++ shp2pgsql.c	1 Apr 2005 16:30:45 -0000
> > @@ -91,6 +91,7 @@
> >  int PIP( Point P, Point* V, int n );
> >  void *safe_malloc(size_t size);
> >  void create_table(void);
> > +int load_data(int num_entities, int trans);
> >  void usage(char *me, int exitcode);
> >  void InsertPoint(void);
> >  void InsertMultiPoint(void);
> > @@ -398,6 +399,8 @@
> >  	}
> >  #endif // defined USE_ICONV
> >  
> > +	printf("BEGIN;\n");
> > +
> >  	if(opt == 'd')
> >  	{
> >  		//---------------Drop the table--------------------------
> > @@ -525,12 +528,10 @@
> >  
> >  	trans=0;
> >  
> > -	printf("BEGIN;\n");
> > -
> >  	//if opt is 'a' do nothing, go straight to making inserts
> > -	if(opt == 'c' || opt == 'd') create_table();
> > +	if(opt == 'c' || opt == 'd' || opt == 'p') create_table();
> >  
> > -	if (dump_format){
> > +	if (dump_format && opt != 'p'){
> >  		if ( schema )
> >  		{
> >  			printf("COPY \"%s\".\"%s\" %s FROM stdin;\n",
> > @@ -543,10 +544,40 @@
> >  		}
> >  	}
> >  
> > -	obj = SHPReadObject(hSHPHandle,0);
> > -	
> > +        if (opt != 'p') { /*only if we do not have prepare mode*/
> > +                j = load_data(num_entities, trans);
> > +        }
> > +        
> > +        free(col_names);
> > +	if(opt != 'a')
> > +	{
> > +		if ( schema )
> > +		{
> > +			printf("\nALTER TABLE ONLY \"%s\".\"%s\" ADD CONSTRAINT \"%s_pkey\" PRIMARY KEY (gid);\n",schema,table,table);
> > +			if(j > 1 && opt != 'p')
> > +			{
> > +				printf("SELECT setval ('\"%s\".\"%s_gid_seq\"', %i, true);\n", schema, table, j-1);
> > +			}
> > +		}
> > +		else
> > +		{
> > +			printf("\nALTER TABLE ONLY \"%s\" ADD CONSTRAINT \"%s_pkey\" PRIMARY KEY (gid);\n",table,table);
> > +			if(j > 1 && opt != 'p'){
> > +				printf("SELECT setval ('\"%s_gid_seq\"', %i, true);\n", table, j-1);
> > +			}
> > +		}
> > +	}
> >  
> > -/**************************************************************
> > +	printf("END;\n"); //End the last transaction
> > +
> > +	return(1);
> > +}//end main()
> > +
> > +
> > +int
> > +load_data(int num_entities, int trans) {
> > +        int j;
> > + /**************************************************************
> >   * 
> >   *   MAIN SHAPE OBJECTS SCAN
> >   * 
> > @@ -635,41 +666,16 @@
> >  				break;
> >  
> >  		}
> > -		
> > +
> >  		SHPDestroyObject(obj);	
> >  
> >  	} // END of MAIN SHAPE OBJECT LOOP
> >  
> > -
> >  	if ((dump_format) ) {
> >  		printf("\\.\n");
> > -
> > -	} 
> > -
> > -	free(col_names);
> > -	if(opt != 'a')
> > -	{
> > -		if ( schema )
> > -		{
> > -			printf("\nALTER TABLE ONLY \"%s\".\"%s\" ADD CONSTRAINT \"%s_pkey\" PRIMARY KEY (gid);\n",schema,table,table);
> > -			if(j > 1)
> > -			{
> > -				printf("SELECT setval ('\"%s\".\"%s_gid_seq\"', %i, true);\n", schema, table, j-1);
> > -			}
> > -		}
> > -		else
> > -		{
> > -			printf("\nALTER TABLE ONLY \"%s\" ADD CONSTRAINT \"%s_pkey\" PRIMARY KEY (gid);\n",table,table);
> > -			if(j > 1){
> > -				printf("SELECT setval ('\"%s_gid_seq\"', %i, true);\n", table, j-1);
> > -			}
> > -		}
> >  	}
> > -
> > -	printf("END;\n"); //End the last transaction
> > -
> > -	return(1);
> > -}//end main()
> > +        return j;
> > +}
> >  
> >  void
> >  create_table(void)
> > @@ -775,13 +781,14 @@
> >  	fprintf(stderr, "OPTIONS:\n");
> >  	fprintf(stderr, "  -s <srid>  Set the SRID field. If not specified it defaults to -1.\n");
> >  	fprintf(stderr, "\n");
> > -	fprintf(stderr, "  (-d|a|c) These are mutually exclusive options:\n");
> > -	fprintf(stderr, "      -d  Drops the table , then recreates it and populates\n");
> > +	fprintf(stderr, "  (-d|a|c|p) These are mutually exclusive options:\n");
> > +	fprintf(stderr, "      -d  Drops the table, then recreates it and populates\n");
> >  	fprintf(stderr, "          it with current shape file data.\n");
> >  	fprintf(stderr, "      -a  Appends shape file into current table, must be\n");
> >  	fprintf(stderr, "          exactly the same table schema.\n");
> >  	fprintf(stderr, "      -c  Creates a new table and populates it, this is the\n");
> >  	fprintf(stderr, "          default if you do not specify any options.\n");
> > +	fprintf(stderr, "      -p  Only creates the table.");
> >  	fprintf(stderr, "\n");
> >  	fprintf(stderr, "  -g <geometry_column> Specify the name of the geometry column\n");
> >  	fprintf(stderr, "     (mostly useful in append mode).\n");
> > @@ -1104,7 +1111,7 @@
> >  	int curindex=0;
> >  	char  *ptr;
> >  
> > -	while ((c = getopt(ARGC, ARGV, "kcdaDs:g:iW:")) != EOF){
> > +	while ((c = getopt(ARGC, ARGV, "kcdapDs:g:iW:")) != EOF){
> >                 switch (c) {
> >                 case 'c':
> >                      if (opt == ' ')
> > @@ -1124,6 +1131,12 @@
> >                      else
> >                           return 0;
> >                      break;
> > +	       case 'p':
> > +                    if (opt == ' ')
> > +                         opt ='p';
> > +                    else
> > +                         return 0;
> > +                    break;
> >  	       case 'D':
> >  		    dump_format =1;
> >                      break;
> 
> > _______________________________________________
> > postgis-devel mailing list
> > postgis-devel at postgis.refractions.net
> > http://postgis.refractions.net/mailman/listinfo/postgis-devel
> 
> _______________________________________________
> postgis-devel mailing list
> postgis-devel at postgis.refractions.net
> http://postgis.refractions.net/mailman/listinfo/postgis-devel



More information about the postgis-devel mailing list