Tue Feb 28 09:27:54 PST 2006
- Previous message: [Slony1-commit] By cbbrowne: Point to new location of RPMs, apply <file> tag to several
- Next message: [Slony1-commit] By darcyb: Fix for Exits without error message if unable to create pid
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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
-
- Previous message: [Slony1-commit] By cbbrowne: Point to new location of RPMs, apply <file> tag to several
- Next message: [Slony1-commit] By darcyb: Fix for Exits without error message if unable to create pid
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
More information about the Slony1-commit mailing list