On 14/04/17 17:33, Peter Eisentraut wrote:
> On 4/14/17 08:49, Petr Jelinek wrote:
>>> Are we prepared to support different schemas in v10? Or should we
>>> disallow it for v10 and add a TODO?
>>>
>>
>> Ah nuts, yes it's supposed to be supported, we seem to not initialize
>> cstate->range_table in tablesync which causes this bug. The CopyState
>> struct is private to copy.c so we can't easily set cstate->range_table
>> externally. I wonder if tablesync should just construct CopyStmt instead
>> of calling the lower level API.
>
> Maybe pass the range_table to BeginCopyFrom so that it can write it into
> cstate?
>
I tried something bit different which seems cleaner to me - use the
pstate->r_table instead of ad-hock locally made up range table and fill
that using standard addRangeTableEntryForRelation. Both in tablesync and
in DoCopy instead of the old coding.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 359c10959f639c1dd3b01f90d459ee4f03ccbd0b Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmo...@pjmodos.net>
Date: Sat, 15 Apr 2017 03:27:32 +0200
Subject: [PATCH] Set range table for CopyFrom in tablesync
This changes the mechanism of how range table is passed to CopyFrom
executor state. We used to generate the range table and one entry for
the relation manually inside DoCopy.Now we use
addRangeTableEntryForRelation to setup the range table and relation
entry for the ParseState which is then passed down by BeginCopyFrom.
---
src/backend/commands/copy.c | 17 +++++++----------
src/backend/replication/logical/tablesync.c | 13 +++++++++++--
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index b5af2be..a786d7b 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -34,6 +34,7 @@
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
+#include "parser/parse_relation.h"
#include "optimizer/clauses.h"
#include "optimizer/planner.h"
#include "nodes/makefuncs.h"
@@ -787,7 +788,6 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
Relation rel;
Oid relid;
RawStmt *query = NULL;
- List *range_table = NIL;
/* Disallow COPY to/from file or program except to superusers. */
if (!pipe && !superuser())
@@ -809,7 +809,6 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
if (stmt->relation)
{
TupleDesc tupDesc;
- AclMode required_access = (is_from ? ACL_INSERT :
ACL_SELECT);
List *attnums;
ListCell *cur;
RangeTblEntry *rte;
@@ -822,12 +821,8 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
relid = RelationGetRelid(rel);
- rte = makeNode(RangeTblEntry);
- rte->rtekind = RTE_RELATION;
- rte->relid = RelationGetRelid(rel);
- rte->relkind = rel->rd_rel->relkind;
- rte->requiredPerms = required_access;
- range_table = list_make1(rte);
+ rte = addRangeTableEntryForRelation(pstate, rel, NULL, false,
false);
+ rte->requiredPerms = (stmt->is_from ? ACL_INSERT : ACL_SELECT);
tupDesc = RelationGetDescr(rel);
attnums = CopyGetAttnums(tupDesc, rel, stmt->attlist);
@@ -841,7 +836,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
else
rte->selectedCols =
bms_add_member(rte->selectedCols, attno);
}
- ExecCheckRTPerms(range_table, true);
+ ExecCheckRTPerms(pstate->p_rtable, true);
/*
* Permission check for row security policies.
@@ -977,7 +972,6 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
cstate = BeginCopyFrom(pstate, rel, stmt->filename,
stmt->is_program,
NULL, stmt->attlist,
stmt->options);
- cstate->range_table = range_table;
*processed = CopyFrom(cstate); /* copy from file to database */
EndCopyFrom(cstate);
}
@@ -2921,6 +2915,9 @@ BeginCopyFrom(ParseState *pstate,
cstate->raw_buf = (char *) palloc(RAW_BUF_SIZE + 1);
cstate->raw_buf_index = cstate->raw_buf_len = 0;
+ /* Assign range table, we'll need it in CopyFrom. */
+ cstate->range_table = pstate->p_rtable;
+
tupDesc = RelationGetDescr(cstate->rel);
attr = tupDesc->attrs;
num_phys_attrs = tupDesc->natts;
diff --git a/src/backend/replication/logical/tablesync.c
b/src/backend/replication/logical/tablesync.c
index d1f2734..9ba2c01 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -93,6 +93,8 @@
#include "commands/copy.h"
+#include "parser/parse_relation.h"
+
#include "replication/logicallauncher.h"
#include "replication/logicalrelation.h"
#include "replication/walreceiver.h"
@@ -648,6 +650,8 @@ copy_table(Relation rel)
StringInfoData cmd;
CopyState cstate;
List *attnamelist;
+ ParseState *pstate;
+ RangeTblEntry *rte;
/* Get the publisher relation info. */
fetch_remote_table_info(get_namespace_name(RelationGetNamespace(rel)),
@@ -674,9 +678,14 @@ copy_table(Relation rel)
copybuf = makeStringInfo();
- /* Create CopyState for ingestion of the data from publisher. */
+ pstate = make_parsestate(NULL);
+ pstate->p_target_relation = rel;
+ rte = addRangeTableEntryForRelation(pstate, pstate->p_target_relation,
+
NULL, false, false);
+ pstate->p_target_rangetblentry = rte;
+
attnamelist = make_copy_attnamelist(relmapentry);
- cstate = BeginCopyFrom(NULL, rel, NULL, false, copy_read_data,
attnamelist, NIL);
+ cstate = BeginCopyFrom(pstate, rel, NULL, false, copy_read_data,
attnamelist, NIL);
/* Do the copy */
(void) CopyFrom(cstate);
--
2.7.4
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
