Skip to content

Commit

Permalink
Remove dead code in ATExecSetDistributedBy
Browse files Browse the repository at this point in the history
This commit is the first step to refactor ATExecSetDistributedBy. Its
main purpose is to remove some dead code in this function and during
the process we find some helper functions can also be simplified so
the simplification is also in this commit.

According to MPP-7770, we should disable changing storage options for now.
It is ugly to just throw an error when encounter `appendonly` option but
without removing the code. In this commit remove all related logic.

Because of with clause can only contain reshuffle|reorganize, we only
new_rel_opts if the table itself is ao|aoco. No need to deduce it from with
clause.

We also remove the unnecessary checks at the start of this function. Because
These checks have been already done in the function `ATPrepCmd`.

Co-authored-by: Shuejie Zhang <shzhang@pivotal.io >
  • Loading branch information
kainwen and zhangjackey authored Nov 22, 2018
1 parent 1136f2f commit 50f2e3b
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 247 deletions.
320 changes: 84 additions & 236 deletions src/backend/commands/tablecmds.c

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion src/backend/nodes/copyfuncs.c
Original file line number Diff line number Diff line change
Expand Up @@ -3318,7 +3318,6 @@ _copySetDistributionCmd(const SetDistributionCmd *from)

COPY_SCALAR_FIELD(backendId);
COPY_NODE_FIELD(relids);
COPY_NODE_FIELD(indexOidMap);
COPY_NODE_FIELD(hiddenTypes);

return newnode;
Expand Down
1 change: 0 additions & 1 deletion src/backend/nodes/equalfuncs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,6 @@ _equalSetDistributionCmd(const SetDistributionCmd *a, const SetDistributionCmd *
{
COMPARE_SCALAR_FIELD(backendId);
COMPARE_NODE_FIELD(relids);
COMPARE_NODE_FIELD(indexOidMap);
COMPARE_NODE_FIELD(hiddenTypes);

return true;
Expand Down
3 changes: 1 addition & 2 deletions src/backend/nodes/outfuncs.c
Original file line number Diff line number Diff line change
Expand Up @@ -2779,13 +2779,12 @@ _outAlterTableCmd(StringInfo str, const AlterTableCmd *node)
}

static void
_outSetDistributionCmd(StringInfo str, const SetDistributionCmd*node)
_outSetDistributionCmd(StringInfo str, const SetDistributionCmd *node)
{
WRITE_NODE_TYPE("SETDISTRIBUTIONCMD");

WRITE_INT_FIELD(backendId);
WRITE_NODE_FIELD(relids);
WRITE_NODE_FIELD(indexOidMap);
WRITE_NODE_FIELD(hiddenTypes);
}

Expand Down
1 change: 0 additions & 1 deletion src/backend/nodes/readfast.c
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,6 @@ _readSetDistributionCmd(void)

READ_INT_FIELD(backendId);
READ_NODE_FIELD(relids);
READ_NODE_FIELD(indexOidMap);
READ_NODE_FIELD(hiddenTypes);

READ_DONE();
Expand Down
1 change: 0 additions & 1 deletion src/backend/nodes/readfuncs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,6 @@ _readSetDistributionCmd(void)

READ_INT_FIELD(backendId);
READ_NODE_FIELD(relids);
READ_NODE_FIELD(indexOidMap);
READ_NODE_FIELD(hiddenTypes);

READ_DONE();
Expand Down
1 change: 0 additions & 1 deletion src/include/nodes/parsenodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1547,7 +1547,6 @@ typedef struct SetDistributionCmd
NodeTag type;
int backendId; /* backend ID on QD */
List *relids; /* oid of relations(partitions) which have related temporary table */
List *indexOidMap; /* the map between relation oid and index oid */
List *hiddenTypes; /* the types need to build for dropped column */
} SetDistributionCmd;

Expand Down
8 changes: 4 additions & 4 deletions src/test/regress/expected/alter_distribution_policy.out
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ select * from distcheck where rel = 'atsdb';

alter table atsdb drop column n;
alter table atsdb set with(appendonly = true, compresslevel = 3);
ERROR: option "appendonly" not supported
ERROR: cannot specify more than one option in WITH clause
select relname, segrelid != 0, reloptions from pg_class, pg_appendonly where pg_class.oid =
'atsdb'::regclass and relid = pg_class.oid;
relname | ?column? | reloptions
Expand Down Expand Up @@ -600,7 +600,7 @@ alter table atsdb set with (fgdfgef = asds);
ERROR: option "fgdfgef" not supported
alter table atsdb set with(reorganize = true, reorganize = false) distributed
randomly;
ERROR: "reorganize" specified more than once
ERROR: cannot specify more than one option in WITH clause
drop table atsdb;
-- check distribution after dropping distribution key column.
create table atsdb (i int, j int, t text, n numeric) distributed by (i, j);
Expand Down Expand Up @@ -823,7 +823,7 @@ NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'c' as
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
insert into mpp5746 select array[i], i from generate_series(1, 100) i;
alter table mpp5746 set with (reorganize=true, appendonly = true);
ERROR: option "appendonly" not supported
ERROR: cannot specify more than one option in WITH clause
select * from mpp5746 order by 1;
c | t
-------+-----
Expand Down Expand Up @@ -1036,7 +1036,7 @@ select * from mpp5746 order by 1;
(100 rows)

alter table mpp5746 set with (reorganize=true, appendonly = false);
ERROR: option "appendonly" not supported
ERROR: cannot specify more than one option in WITH clause
select * from mpp5746 order by 1;
c
-------
Expand Down

0 comments on commit 50f2e3b

Please sign in to comment.