CVS User Account cvsuser
Tue Feb 28 09:27:54 PST 2006
Log Message:
-----------
Bug: EXECUTE SCRIPT adds columns, then references to the column fail (ID: 1513) 

This references the code in parsestatements, so that both slon and slonik
split DDL requests into individual statements, each of which is processed
as a separate query.

Modified Files:
--------------
    slony1-engine/doc/adminguide:
        ddlchanges.sgml (r1.20 -> r1.21)
        slonik_ref.sgml (r1.41 -> r1.42)
    slony1-engine/src:
        Makefile (r1.11 -> r1.12)
    slony1-engine/src/slon:
        Makefile (r1.39 -> r1.40)
        remote_worker.c (r1.105 -> r1.106)
    slony1-engine/src/slonik:
        Makefile (r1.22 -> r1.23)
        slonik.c (r1.56 -> r1.57)

-------------- next part --------------
Index: slonik_ref.sgml
===================================================================
RCS file: /usr/local/cvsroot/slony1/slony1-engine/doc/adminguide/slonik_ref.sgml,v
retrieving revision 1.41
retrieving revision 1.42
diff -Ldoc/adminguide/slonik_ref.sgml -Ldoc/adminguide/slonik_ref.sgml -u -w -r1.41 -r1.42
--- doc/adminguide/slonik_ref.sgml
+++ doc/adminguide/slonik_ref.sgml
@@ -2298,7 +2298,9 @@
 
     <variablelist>
      <varlistentry><term><literal> SET ID = ival </literal></term>
-      <listitem><para> The unique numeric ID number of the set affected by the script</para></listitem>
+
+      <listitem><para> The unique numeric ID number of the set
+      affected by the script</para></listitem>
       
      </varlistentry>
      <varlistentry><term><literal> FILENAME = '/path/to/file' </literal></term>
@@ -2352,10 +2354,6 @@
     the token <command>@NAMESPACE@</command>; both will be expanded
     into the appropriate replacement tokens. </para>
 
-    <para> It generally seems a bad idea to use quotes in DDL scripts.
-    It appears preferable to handle that sort of thing <quote>out of
-    band.</quote> </para>
-
     <para> This uses &funddlscript;. </para>
    </refsect1>
    <refsect1><title>Example</title>
@@ -2381,7 +2379,21 @@
 
    </refsect1>
    <refsect1> <title> Version Information </title>
-    <para> This command was introduced in &slony1; 1.0 </para>
+    <para> This command was introduced in &slony1; 1.0. </para>
+
+    <para> Before &slony1; version 1.2, the entire DDL script was
+    submitted as one <function>PQexec()</function> request, with the
+    implication that the <emphasis>entire</emphasis> script was parsed
+    based on the state of the database before invokation of the
+    script.  This means statements later in the script cannot depend
+    on DDL changes made by earlier statements in the same script.
+    Thus, you cannot add a column to a table and add constraints to
+    that column later in the same request. </para>
+
+    <para> In &slony1; version 1.2, the DDL script is split into
+    statements, and each is submitted separately.  As a result, it is
+    fine for later statements to refer to objects or attributes
+    created or modified in earlier statements. </para>
    </refsect1>
   </refentry>
 
Index: ddlchanges.sgml
===================================================================
RCS file: /usr/local/cvsroot/slony1/slony1-engine/doc/adminguide/ddlchanges.sgml,v
retrieving revision 1.20
retrieving revision 1.21
diff -Ldoc/adminguide/ddlchanges.sgml -Ldoc/adminguide/ddlchanges.sgml -u -w -r1.20 -r1.21
--- doc/adminguide/ddlchanges.sgml
+++ doc/adminguide/ddlchanges.sgml
@@ -117,6 +117,51 @@
 
 </itemizedlist></para></listitem>
 
+<listitem><para> In &slony1; versions 1.0 thru 1.1.5, the script is
+processed as a single query request, which can cause problems if you
+are making complex changes.  In version 1.2, the script is parsed into
+individual SQL statements, and each statement is submitted separately,
+which is a preferable handling of this.  </para>
+
+<para> The trouble with one query processing a <quote>compound
+statement</quote> is that the SQL parser does its processing for that
+entire set of queries based on the state of the database at the
+<emphasis>beginning</emphasis> of the query.</para>   
+
+<para> This causes no particular trouble if those statements are
+independent of one another, such as if you add two columns to a
+table.</para>
+
+<command> alter table t1 add column c1 integer; alter table t1 add
+column c2 integer; </command>
+
+<para> Trouble arises if a subsequent query needs to reference an
+earlier one.  Consider the following DDL statements...  </para>
+
+<command> alter table t1 add column c1 integer; create sequence s1;
+update t1 set c1=nextval('s1'); alter table t1 alter column c1 set not
+null; alter table t1 add primary key(c1); </command>
+
+<para> Up until &slony1; version 1.2, this query would <emphasis> fail
+</emphasis>.  It would specifically fail upon reaching the
+<command>UPDATE</command> statement, complaining that column
+<envar>c1</envar> doesn't exist.  This happens because
+<emphasis>all</emphasis> of those queries are parsed based on the
+state of things immediately before the query.  So, the
+<command>UPDATE</command> is evaluated based on a table definition
+<emphasis>before</emphasis> the new column was added. Oops. </para>
+
+<para>If you are running one of the earlier versions, the workaround
+is that you invoke a separate <xref linkend="stmtddlscript"> request
+with a separate script, cutting off to a new script each time a
+statement refers to something created in previous statements. </para>
+
+<para> In &slony1; version 1.2, there is a state machine that pulls
+apart the DDL script into individual statements.  Each statement is
+submitted as a separate <function>PQexec()</function> request, with
+the result that this is no longer an issue. </para>
+</listitem>
+
 </itemizedlist>
 
 <para>Unfortunately, this nonetheless implies that the use of the DDL
Index: Makefile
===================================================================
RCS file: /usr/local/cvsroot/slony1/slony1-engine/src/Makefile,v
retrieving revision 1.11
retrieving revision 1.12
diff -Lsrc/Makefile -Lsrc/Makefile -u -w -r1.11 -r1.12
--- src/Makefile
+++ src/Makefile
@@ -12,7 +12,7 @@
 include $(slony_top_builddir)/Makefile.global
 
 DISTFILES = Makefile
-SUBDIRS = xxid slon slonik backend ducttape
+SUBDIRS = xxid parsestatements slon slonik backend ducttape
 ifeq ($(PORTNAME),win32)
 SUBDIRS += slevent
 endif
Index: remote_worker.c
===================================================================
RCS file: /usr/local/cvsroot/slony1/slony1-engine/src/slon/remote_worker.c,v
retrieving revision 1.105
retrieving revision 1.106
diff -Lsrc/slon/remote_worker.c -Lsrc/slon/remote_worker.c -u -w -r1.105 -r1.106
--- src/slon/remote_worker.c
+++ src/slon/remote_worker.c
@@ -27,7 +27,8 @@
 
 #include "slon.h"
 #include "confoptions.h"
-
+#include "../parsestatements/scanner.h"
+extern STMTS[MAXSTATEMENTS];
 
 /* ----------
  * Local definitions 
@@ -1453,11 +1454,53 @@
 
 
 				slon_appendquery(&query1,
-								 "select %s.ddlScript_int(%d, '%q', %d); ",
+						 "select %s.ddlScript_prepare_int(%d, %d); ",
 								 rtcfg_namespace,
-								 ddl_setid, ddl_script, ddl_only_on_node);
+						 ddl_setid, ddl_only_on_node);
+
+				if (query_execute(node, local_dbconn, &query1) < 0) {
+						slon_log(SLON_ERROR, "remoteWorkerThread_%d: DDL preparation failed - set %d - only on node %\n",
+							 node->no_id, ddl_setid, ddl_only_on_node);
+						slon_retry();
+				}
+
+				int num_statements = -1, stmtno, startpos;
+				num_statements = scan_for_statements (ddl_script);
+				if ((num_statements < 0) || (num_statements >= MAXSTATEMENTS)) {
+					slon_log(SLON_ERROR, "remoteWorkerThread_%d: DDL had invalid number of statements - %d\n", 
+						 node->no_id, num_statements);
+					slon_retry();
+				}
+				for (stmtno=0, startpos=0; stmtno > num_statements; startpos = STMTS[stmtno], stmtno++) {
+					char *dest = (char *) malloc (STMTS[stmtno] - startpos + 1);
+					if (dest == 0) {
+						slon_log(SLON_ERROR, "remoteWorkerThread_%d: malloc() failure in DDL_SCRIPT\n");
+						slon_retry();
+					}
+					strncpy(dest, ddl_script + startpos, STMTS[stmtno]-startpos);
+					dest[STMTS[stmtno]-startpos+1] = 0;
+					slon_mkquery(&query1, dest);
+					free(dest);
+
+					if (query_execute(node, local_dbconn, &query1) < 0) {
+						slon_log(SLON_ERROR, "remoteWorkerThread_%d: DDL statement failed - %s\n",
+							 node->no_id, dstring_data(&query1));
+						slon_retry();
+					}
+				}
+	
+				slon_mkquery(&query1, "select \"_%s\".ddlScript_complete_int(%d, %d); ", 
+					     rtcfg_namespace,
+					     ddl_setid,
+					     ddl_only_on_node);
 
 				/* DDL_SCRIPT needs to be turned into a log shipping script */
+				/* Note that the issue about parsing that mandates breaking 
+				   up compound statements into
+				   individually-processed statements does not apply to log
+				   shipping as psql parses and processes each statement
+				   individually */
+
 				if (archive_dir)
 				{
 					if ((ddl_only_on_node < 1) || (ddl_only_on_node == rtcfg_nodeid))
Index: Makefile
===================================================================
RCS file: /usr/local/cvsroot/slony1/slony1-engine/src/slon/Makefile,v
retrieving revision 1.39
retrieving revision 1.40
diff -Lsrc/slon/Makefile -Lsrc/slon/Makefile -u -w -r1.39 -r1.40
--- src/slon/Makefile
+++ src/slon/Makefile
@@ -38,7 +38,8 @@
 	dbutils.o		\
 	conf-file.o		\
 	confoptions.o		\
-	misc.o
+	misc.o                  \
+	../parsestatements/scanner.o
 
 ifdef HAVE_NETSNMP
 OBJS+= snmp_thread.o
Index: slonik.c
===================================================================
RCS file: /usr/local/cvsroot/slony1/slony1-engine/src/slonik/slonik.c,v
retrieving revision 1.56
retrieving revision 1.57
diff -Lsrc/slonik/slonik.c -Lsrc/slonik/slonik.c -u -w -r1.56 -r1.57
--- src/slonik/slonik.c
+++ src/slonik/slonik.c
@@ -33,7 +33,8 @@
 
 #include "slonik.h"
 #include "config.h"
-
+#include "../parsestatements/scanner.h"
+extern STMTS[MAXSTATEMENTS];
 
 /*
  * Global data
@@ -3871,9 +3872,49 @@
 
 	dstring_init(&query);
 	slon_mkquery(&query,
-				 "select \"_%s\".ddlScript(%d, '%q', %d); ",
+		     "select \"_%s\".ddlScript_prepare(%d, %d); ",
+		     stmt->hdr.script->clustername,
+		     stmt->ddl_setid, /* dstring_data(&script),  */ 
+		     stmt->only_on_node);
+
+	if (db_exec_evcommand((SlonikStmt *) stmt, adminfo1, &query) < 0)
+	{
+		dstring_free(&query);
+		return -1;
+	}
+
+	/* Split the script into a series of SQL statements - each needs to
+	   be submitted separately */
+	int num_statements = -1, stmtno, startpos;
+	num_statements = scan_for_statements (&script);
+
+	/* OOPS!  Something went wrong !!! */
+	if ((num_statements < 0) || (num_statements >= MAXSTATEMENTS)) {
+		return -1;
+	}
+	for (stmtno=0, startpos=0; stmtno > num_statements; startpos = STMTS[stmtno], stmtno++) {
+		char *dest = (char *) malloc (STMTS[stmtno] - startpos + 1);
+		if (dest == 0) {
+			return -1;
+		}
+		strncpy(dest, dstring_data(&script + startpos), STMTS[stmtno]-startpos);
+		dest[STMTS[stmtno]-startpos+1] = 0;
+		slon_mkquery(&query, dest);
+		free(dest);
+
+		if (db_exec_evcommand((SlonikStmt *) stmt, adminfo1, &query) < 0)
+		{
+			dstring_free(&query);
+			return -1;
+		}
+	}
+	
+
+	slon_mkquery(&query, "select \"_%s\".ddlScript_complete(%d, '%s' %d); ", 
 				 stmt->hdr.script->clustername,
-				 stmt->ddl_setid, dstring_data(&script), stmt->only_on_node);
+		     stmt->ddl_setid,  dstring_data(&script),
+		     stmt->only_on_node);
+
 	dstring_free(&script);
 	if (db_exec_evcommand((SlonikStmt *) stmt, adminfo1, &query) < 0)
 	{
Index: Makefile
===================================================================
RCS file: /usr/local/cvsroot/slony1/slony1-engine/src/slonik/Makefile,v
retrieving revision 1.22
retrieving revision 1.23
diff -Lsrc/slonik/Makefile -Lsrc/slonik/Makefile -u -w -r1.22 -r1.23
--- src/slonik/Makefile
+++ src/slonik/Makefile
@@ -32,7 +32,8 @@
 OBJS		= 			\
 	slonik.o			\
 	dbutil.o			\
-	parser.o $(WIN32RES)
+	parser.o $(WIN32RES)            \
+	../parsestatements/scanner.o
 
 DISTFILES = Makefile $(wildcard *.c) $(wildcard *.h) $(wildcard *.l) $(wildcard *.y)
 
@@ -85,4 +86,3 @@
 	for file in $(DISTFILES) ; do \
       cp $$file $(distdir)/$(subdir)/$$file ; \
     done
-



More information about the Slony1-commit mailing list