Commit b3aaf908 authored by Tom Lane's avatar Tom Lane

Rearrange planner to save the whole PlannerInfo (subroot) for a subquery.

Formerly, set_subquery_pathlist and other creators of plans for subqueries
saved only the rangetable and rowMarks lists from the lower-level
PlannerInfo.  But there's no reason not to remember the whole PlannerInfo,
and indeed this turns out to simplify matters in a number of places.

The immediate reason for doing this was so that the subroot will still be
accessible when we're trying to extract column statistics out of an
already-planned subquery.  But now that I've done it, it seems like a good
code-beautification effort in its own right.

I also chose to get rid of the transient subrtable and subrowmark fields in
SubqueryScan nodes, in favor of having setrefs.c look up the subquery's
RelOptInfo.  That required changing all the APIs in setrefs.c to pass
PlannerInfo not PlannerGlobal, which was a large but quite mechanical
transformation.

One side-effect not foreseen at the beginning is that this finally broke
inheritance_planner's assumption that replanning the same subquery RTE N
times would necessarily give interchangeable results each time.  That
assumption was always pretty risky, but now we really have to make a
separate RTE for each instance so that there's a place to carry the
separate subroots.
parent 42ad992f
......@@ -99,14 +99,9 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags)
/* check for unsupported flags */
Assert(!(eflags & EXEC_FLAG_MARK));
/*
* SubqueryScan should not have any "normal" children. Also, if planner
* left anything in subrtable/subrowmark, it's fishy.
*/
/* SubqueryScan should not have any "normal" children */
Assert(outerPlan(node) == NULL);
Assert(innerPlan(node) == NULL);
Assert(node->subrtable == NIL);
Assert(node->subrowmark == NIL);
/*
* create state structure
......
......@@ -456,8 +456,6 @@ _copySubqueryScan(SubqueryScan *from)
* copy remainder of node
*/
COPY_NODE_FIELD(subplan);
COPY_NODE_FIELD(subrtable);
COPY_NODE_FIELD(subrowmark);
return newnode;
}
......
......@@ -489,8 +489,6 @@ _outSubqueryScan(StringInfo str, SubqueryScan *node)
_outScanInfo(str, (Scan *) node);
WRITE_NODE_FIELD(subplan);
WRITE_NODE_FIELD(subrtable);
WRITE_NODE_FIELD(subrowmark);
}
static void
......@@ -1656,8 +1654,7 @@ _outPlannerGlobal(StringInfo str, PlannerGlobal *node)
/* NB: this isn't a complete set of fields */
WRITE_NODE_FIELD(paramlist);
WRITE_NODE_FIELD(subplans);
WRITE_NODE_FIELD(subrtables);
WRITE_NODE_FIELD(subrowmarks);
WRITE_NODE_FIELD(subroots);
WRITE_BITMAPSET_FIELD(rewindPlanIDs);
WRITE_NODE_FIELD(finalrtable);
WRITE_NODE_FIELD(finalrowmarks);
......@@ -1734,8 +1731,7 @@ _outRelOptInfo(StringInfo str, RelOptInfo *node)
WRITE_UINT_FIELD(pages);
WRITE_FLOAT_FIELD(tuples, "%.0f");
WRITE_NODE_FIELD(subplan);
WRITE_NODE_FIELD(subrtable);
WRITE_NODE_FIELD(subrowmark);
WRITE_NODE_FIELD(subroot);
WRITE_NODE_FIELD(baserestrictinfo);
WRITE_NODE_FIELD(joininfo);
WRITE_BOOL_FIELD(has_eclass_joins);
......
......@@ -791,11 +791,10 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
root,
false, tuple_fraction,
&subroot);
rel->subrtable = subroot->parse->rtable;
rel->subrowmark = subroot->rowMarks;
rel->subroot = subroot;
/* Mark rel with estimated output rows, width, etc */
set_subquery_size_estimates(root, rel, subroot);
set_subquery_size_estimates(root, rel);
/* Convert subquery pathkeys to outer representation */
pathkeys = convert_subquery_pathkeys(root, rel, subroot->query_pathkeys);
......
......@@ -3221,9 +3221,9 @@ set_joinrel_size_estimates(PlannerInfo *root, RelOptInfo *rel,
* We set the same fields as set_baserel_size_estimates.
*/
void
set_subquery_size_estimates(PlannerInfo *root, RelOptInfo *rel,
PlannerInfo *subroot)
set_subquery_size_estimates(PlannerInfo *root, RelOptInfo *rel)
{
PlannerInfo *subroot = rel->subroot;
RangeTblEntry *rte;
ListCell *lc;
......
......@@ -1557,9 +1557,7 @@ create_subqueryscan_plan(PlannerInfo *root, Path *best_path,
scan_plan = make_subqueryscan(tlist,
scan_clauses,
scan_relid,
best_path->parent->subplan,
best_path->parent->subrtable,
best_path->parent->subrowmark);
best_path->parent->subplan);
copy_path_costsize(&scan_plan->scan.plan, best_path);
......@@ -2931,9 +2929,7 @@ SubqueryScan *
make_subqueryscan(List *qptlist,
List *qpqual,
Index scanrelid,
Plan *subplan,
List *subrtable,
List *subrowmark)
Plan *subplan)
{
SubqueryScan *node = makeNode(SubqueryScan);
Plan *plan = &node->scan.plan;
......@@ -2952,8 +2948,6 @@ make_subqueryscan(List *qptlist,
plan->righttree = NULL;
node->scan.scanrelid = scanrelid;
node->subplan = subplan;
node->subrtable = subrtable;
node->subrowmark = subrowmark;
return node;
}
......
......@@ -98,7 +98,6 @@ query_planner(PlannerInfo *root, List *tlist,
Path *cheapestpath;
Path *sortedpath;
Index rti;
ListCell *lc;
double total_pages;
/* Make tuple_fraction, limit_tuples accessible to lower-level routines */
......@@ -128,15 +127,11 @@ query_planner(PlannerInfo *root, List *tlist,
}
/*
* Init planner lists to empty, and set up the array to hold RelOptInfos
* for "simple" rels.
* Init planner lists to empty.
*
* NOTE: append_rel_list was set up by subquery_planner, so do not touch
* here; eq_classes and minmax_aggs may contain data already, too.
*/
root->simple_rel_array_size = list_length(parse->rtable) + 1;
root->simple_rel_array = (RelOptInfo **)
palloc0(root->simple_rel_array_size * sizeof(RelOptInfo *));
root->join_rel_list = NIL;
root->join_rel_hash = NULL;
root->join_rel_level = NULL;
......@@ -151,17 +146,10 @@ query_planner(PlannerInfo *root, List *tlist,
/*
* Make a flattened version of the rangetable for faster access (this is
* OK because the rangetable won't change any more).
* OK because the rangetable won't change any more), and set up an
* empty array for indexing base relations.
*/
root->simple_rte_array = (RangeTblEntry **)
palloc0(root->simple_rel_array_size * sizeof(RangeTblEntry *));
rti = 1;
foreach(lc, parse->rtable)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
root->simple_rte_array[rti++] = rte;
}
setup_simple_rel_arrays(root);
/*
* Construct RelOptInfo nodes for all base relations in query, and
......
This diff is collapsed.
This diff is collapsed.
......@@ -52,8 +52,7 @@ typedef struct finalize_primnode_context
} finalize_primnode_context;
static Node *build_subplan(PlannerInfo *root, Plan *plan,
List *rtable, List *rowmarks,
static Node *build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot,
SubLinkType subLinkType, Node *testexpr,
bool adjust_testexpr, bool unknownEqFalse);
static List *generate_subquery_params(PlannerInfo *root, List *tlist,
......@@ -389,8 +388,7 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType,
&subroot);
/* And convert to SubPlan or InitPlan format. */
result = build_subplan(root, plan,
subroot->parse->rtable, subroot->rowMarks,
result = build_subplan(root, plan, subroot,
subLinkType, testexpr, true, isTopQual);
/*
......@@ -430,9 +428,7 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType,
AlternativeSubPlan *asplan;
/* OK, convert to SubPlan format. */
hashplan = (SubPlan *) build_subplan(root, plan,
subroot->parse->rtable,
subroot->rowMarks,
hashplan = (SubPlan *) build_subplan(root, plan, subroot,
ANY_SUBLINK, newtestexpr,
false, true);
/* Check we got what we expected */
......@@ -460,7 +456,7 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType,
* as explained in the comments for make_subplan.
*/
static Node *
build_subplan(PlannerInfo *root, Plan *plan, List *rtable, List *rowmarks,
build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot,
SubLinkType subLinkType, Node *testexpr,
bool adjust_testexpr, bool unknownEqFalse)
{
......@@ -644,11 +640,10 @@ build_subplan(PlannerInfo *root, Plan *plan, List *rtable, List *rowmarks,
}
/*
* Add the subplan and its rtable to the global lists.
* Add the subplan and its PlannerInfo to the global lists.
*/
root->glob->subplans = lappend(root->glob->subplans, plan);
root->glob->subrtables = lappend(root->glob->subrtables, rtable);
root->glob->subrowmarks = lappend(root->glob->subrowmarks, rowmarks);
root->glob->subroots = lappend(root->glob->subroots, subroot);
splan->plan_id = list_length(root->glob->subplans);
if (isInitPlan)
......@@ -1018,13 +1013,10 @@ SS_process_ctes(PlannerInfo *root)
splan->setParam = list_make1_int(prm->paramid);
/*
* Add the subplan and its rtable to the global lists.
* Add the subplan and its PlannerInfo to the global lists.
*/
root->glob->subplans = lappend(root->glob->subplans, plan);
root->glob->subrtables = lappend(root->glob->subrtables,
subroot->parse->rtable);
root->glob->subrowmarks = lappend(root->glob->subrowmarks,
subroot->rowMarks);
root->glob->subroots = lappend(root->glob->subroots, subroot);
splan->plan_id = list_length(root->glob->subplans);
root->init_plans = lappend(root->init_plans, splan);
......@@ -2406,14 +2398,10 @@ SS_make_initplan_from_plan(PlannerInfo *root, Plan *plan,
SS_finalize_plan(root, plan, false);
/*
* Add the subplan and its rtable to the global lists.
* Add the subplan and its PlannerInfo to the global lists.
*/
root->glob->subplans = lappend(root->glob->subplans,
plan);
root->glob->subrtables = lappend(root->glob->subrtables,
root->parse->rtable);
root->glob->subrowmarks = lappend(root->glob->subrowmarks,
root->rowMarks);
root->glob->subplans = lappend(root->glob->subplans, plan);
root->glob->subroots = lappend(root->glob->subroots, root);
/*
* Create a SubPlan node and add it to the outer list of InitPlans. Note
......
......@@ -130,6 +130,7 @@ plan_set_operations(PlannerInfo *root, double tuple_fraction,
Query *parse = root->parse;
SetOperationStmt *topop = (SetOperationStmt *) parse->setOperations;
Node *node;
RangeTblEntry *leftmostRTE;
Query *leftmostQuery;
Assert(topop && IsA(topop, SetOperationStmt));
......@@ -142,6 +143,13 @@ plan_set_operations(PlannerInfo *root, double tuple_fraction,
Assert(parse->windowClause == NIL);
Assert(parse->distinctClause == NIL);
/*
* We'll need to build RelOptInfos for each of the leaf subqueries,
* which are RTE_SUBQUERY rangetable entries in this Query. Prepare the
* index arrays for that.
*/
setup_simple_rel_arrays(root);
/*
* Find the leftmost component Query. We need to use its column names for
* all generated tlists (else SELECT INTO won't work right).
......@@ -150,8 +158,8 @@ plan_set_operations(PlannerInfo *root, double tuple_fraction,
while (node && IsA(node, SetOperationStmt))
node = ((SetOperationStmt *) node)->larg;
Assert(node && IsA(node, RangeTblRef));
leftmostQuery = rt_fetch(((RangeTblRef *) node)->rtindex,
parse->rtable)->subquery;
leftmostRTE = root->simple_rte_array[((RangeTblRef *) node)->rtindex];
leftmostQuery = leftmostRTE->subquery;
Assert(leftmostQuery != NULL);
/*
......@@ -206,14 +214,22 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
if (IsA(setOp, RangeTblRef))
{
RangeTblRef *rtr = (RangeTblRef *) setOp;
RangeTblEntry *rte = rt_fetch(rtr->rtindex, root->parse->rtable);
RangeTblEntry *rte = root->simple_rte_array[rtr->rtindex];
Query *subquery = rte->subquery;
RelOptInfo *rel;
PlannerInfo *subroot;
Plan *subplan,
*plan;
Assert(subquery != NULL);
/*
* We need to build a RelOptInfo for each leaf subquery. This isn't
* used for anything here, but it carries the subroot data structures
* forward to setrefs.c processing.
*/
rel = build_simple_rel(root, rtr->rtindex, RELOPT_BASEREL);
/*
* Generate plan for primitive subquery
*/
......@@ -222,6 +238,10 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
false, tuple_fraction,
&subroot);
/* Save subroot and subplan in RelOptInfo for setrefs.c */
rel->subplan = subplan;
rel->subroot = subroot;
/*
* Estimate number of groups if caller wants it. If the subquery used
* grouping or aggregation, its output is probably mostly unique
......@@ -250,9 +270,7 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
refnames_tlist),
NIL,
rtr->rtindex,
subplan,
subroot->parse->rtable,
subroot->rowMarks);
subplan);
/*
* We don't bother to determine the subquery's output ordering since
......
......@@ -57,7 +57,7 @@ typedef struct
typedef struct
{
ParamListInfo boundParams;
PlannerGlobal *glob;
PlannerInfo *root;
List *active_fns;
Node *case_val;
bool estimate;
......@@ -2058,15 +2058,10 @@ eval_const_expressions(PlannerInfo *root, Node *node)
eval_const_expressions_context context;
if (root)
{
context.boundParams = root->glob->boundParams; /* bound Params */
context.glob = root->glob; /* for inlined-function dependencies */
}
else
{
context.boundParams = NULL;
context.glob = NULL;
}
context.root = root; /* for inlined-function dependencies */
context.active_fns = NIL; /* nothing being recursively simplified */
context.case_val = NULL; /* no CASE being examined */
context.estimate = false; /* safe transformations only */
......@@ -2097,7 +2092,7 @@ estimate_expression_value(PlannerInfo *root, Node *node)
context.boundParams = root->glob->boundParams; /* bound Params */
/* we do not need to mark the plan as depending on inlined functions */
context.glob = NULL;
context.root = NULL;
context.active_fns = NIL; /* nothing being recursively simplified */
context.case_val = NULL; /* no CASE being examined */
context.estimate = true; /* unsafe transformations OK */
......@@ -4123,8 +4118,8 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
* Since there is now no trace of the function in the plan tree, we must
* explicitly record the plan's dependency on the function.
*/
if (context->glob)
record_plan_function_dependency(context->glob, funcid);
if (context->root)
record_plan_function_dependency(context->root, funcid);
/*
* Recursively try to simplify the modified expression. Here we must add
......@@ -4559,7 +4554,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
* Since there is now no trace of the function in the plan tree, we must
* explicitly record the plan's dependency on the function.
*/
record_plan_function_dependency(root->glob, func_oid);
record_plan_function_dependency(root, func_oid);
return querytree;
......
......@@ -45,6 +45,35 @@ static List *subbuild_joinrel_joinlist(RelOptInfo *joinrel,
List *new_joininfo);
/*
* setup_simple_rel_arrays
* Prepare the arrays we use for quickly accessing base relations.
*/
void
setup_simple_rel_arrays(PlannerInfo *root)
{
Index rti;
ListCell *lc;
/* Arrays are accessed using RT indexes (1..N) */
root->simple_rel_array_size = list_length(root->parse->rtable) + 1;
/* simple_rel_array is initialized to all NULLs */
root->simple_rel_array = (RelOptInfo **)
palloc0(root->simple_rel_array_size * sizeof(RelOptInfo *));
/* simple_rte_array is an array equivalent of the rtable list */
root->simple_rte_array = (RangeTblEntry **)
palloc0(root->simple_rel_array_size * sizeof(RangeTblEntry *));
rti = 1;
foreach(lc, root->parse->rtable)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
root->simple_rte_array[rti++] = rte;
}
}
/*
* build_simple_rel
* Construct a new RelOptInfo for a base relation or 'other' relation.
......@@ -81,8 +110,7 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptKind reloptkind)
rel->pages = 0;
rel->tuples = 0;
rel->subplan = NULL;
rel->subrtable = NIL;
rel->subrowmark = NIL;
rel->subroot = NULL;
rel->baserestrictinfo = NIL;
rel->baserestrictcost.startup = 0;
rel->baserestrictcost.per_tuple = 0;
......@@ -335,8 +363,7 @@ build_join_rel(PlannerInfo *root,
joinrel->pages = 0;
joinrel->tuples = 0;
joinrel->subplan = NULL;
joinrel->subrtable = NIL;
joinrel->subrowmark = NIL;
joinrel->subroot = NULL;
joinrel->baserestrictinfo = NIL;
joinrel->baserestrictcost.startup = 0;
joinrel->baserestrictcost.per_tuple = 0;
......
......@@ -16,6 +16,7 @@
#include "catalog/pg_type.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "nodes/plannodes.h"
#include "optimizer/clauses.h"
#include "parser/parse_coerce.h"
#include "parser/parse_relation.h"
......@@ -375,6 +376,7 @@ OffsetVarNodes_walker(Node *node, OffsetVarNodes_context *context)
/* fall through to examine children */
}
/* Shouldn't need to handle other planner auxiliary nodes here */
Assert(!IsA(node, PlanRowMark));
Assert(!IsA(node, SpecialJoinInfo));
Assert(!IsA(node, PlaceHolderInfo));
Assert(!IsA(node, MinMaxAggInfo));
......@@ -529,6 +531,19 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
}
/* fall through to examine children */
}
if (IsA(node, PlanRowMark))
{
PlanRowMark *rowmark = (PlanRowMark *) node;
if (context->sublevels_up == 0)
{
if (rowmark->rti == context->rt_index)
rowmark->rti = context->new_index;
if (rowmark->prti == context->rt_index)
rowmark->prti = context->new_index;
}
return false;
}
if (IsA(node, AppendRelInfo))
{
AppendRelInfo *appinfo = (AppendRelInfo *) node;
......@@ -810,6 +825,7 @@ rangeTableEntry_used_walker(Node *node,
}
/* Shouldn't need to handle planner auxiliary nodes here */
Assert(!IsA(node, PlaceHolderVar));
Assert(!IsA(node, PlanRowMark));
Assert(!IsA(node, SpecialJoinInfo));
Assert(!IsA(node, AppendRelInfo));
Assert(!IsA(node, PlaceHolderInfo));
......
......@@ -380,18 +380,12 @@ typedef struct TidScan
* the generic lefttree field as you might expect. This is because we do
* not want plan-tree-traversal routines to recurse into the subplan without
* knowing that they are changing Query contexts.
*
* Note: subrtable is used just to carry the subquery rangetable from
* createplan.c to setrefs.c; it should always be NIL by the time the
* executor sees the plan. Similarly for subrowmark.
* ----------------
*/
typedef struct SubqueryScan
{
Scan scan;
Plan *subplan;
List *subrtable; /* temporary workspace for planner */
List *subrowmark; /* temporary workspace for planner */
} SubqueryScan;
/* ----------------
......
......@@ -79,9 +79,7 @@ typedef struct PlannerGlobal
List *subplans; /* Plans for SubPlan nodes */
List *subrtables; /* Rangetables for SubPlan nodes */
List *subrowmarks; /* PlanRowMarks for SubPlan nodes */
List *subroots; /* PlannerInfos for SubPlan nodes */
Bitmapset *rewindPlanIDs; /* indices of subplans that require REWIND */
......@@ -322,10 +320,9 @@ typedef struct PlannerInfo
* pages - number of disk pages in relation (zero if not a table)
* tuples - number of tuples in relation (not considering restrictions)
* subplan - plan for subquery (NULL if it's not a subquery)
* subrtable - rangetable for subquery (NIL if it's not a subquery)
* subrowmark - rowmarks for subquery (NIL if it's not a subquery)
* subroot - PlannerInfo for subquery (NULL if it's not a subquery)
*
* Note: for a subquery, tuples and subplan are not set immediately
* Note: for a subquery, tuples, subplan, subroot are not set immediately
* upon creation of the RelOptInfo object; they are filled in when
* set_base_rel_pathlist processes the object.
*
......@@ -408,8 +405,7 @@ typedef struct RelOptInfo
BlockNumber pages;
double tuples;
struct Plan *subplan; /* if subquery */
List *subrtable; /* if subquery */
List *subrowmark; /* if subquery */
PlannerInfo *subroot; /* if subquery */
/* used by various scans and joins: */
List *baserestrictinfo; /* RestrictInfo structures (if base
......
......@@ -121,8 +121,7 @@ extern void set_joinrel_size_estimates(PlannerInfo *root, RelOptInfo *rel,
RelOptInfo *inner_rel,
SpecialJoinInfo *sjinfo,
List *restrictlist);
extern void set_subquery_size_estimates(PlannerInfo *root, RelOptInfo *rel,
PlannerInfo *subroot);
extern void set_subquery_size_estimates(PlannerInfo *root, RelOptInfo *rel);
extern void set_function_size_estimates(PlannerInfo *root, RelOptInfo *rel);
extern void set_values_size_estimates(PlannerInfo *root, RelOptInfo *rel);
extern void set_cte_size_estimates(PlannerInfo *root, RelOptInfo *rel,
......
......@@ -96,6 +96,7 @@ extern HashPath *create_hashjoin_path(PlannerInfo *root,
/*
* prototypes for relnode.c
*/
extern void setup_simple_rel_arrays(PlannerInfo *root);
extern RelOptInfo *build_simple_rel(PlannerInfo *root, int relid,
RelOptKind reloptkind);
extern RelOptInfo *find_base_rel(PlannerInfo *root, int relid);
......
......@@ -41,8 +41,7 @@ extern Plan *optimize_minmax_aggregates(PlannerInfo *root, List *tlist,
*/
extern Plan *create_plan(PlannerInfo *root, Path *best_path);
extern SubqueryScan *make_subqueryscan(List *qptlist, List *qpqual,
Index scanrelid, Plan *subplan,
List *subrtable, List *subrowmark);
Index scanrelid, Plan *subplan);
extern Append *make_append(List *appendplans, List *tlist);
extern RecursiveUnion *make_recursive_union(List *tlist,
Plan *lefttree, Plan *righttree, int wtParam,
......@@ -118,18 +117,15 @@ extern List *remove_useless_joins(PlannerInfo *root, List *joinlist);
/*
* prototypes for plan/setrefs.c
*/
extern Plan *set_plan_references(PlannerGlobal *glob,
Plan *plan,
List *rtable,
List *rowmarks);
extern List *set_returning_clause_references(PlannerGlobal *glob,
extern Plan *set_plan_references(PlannerInfo *root, Plan *plan);
extern List *set_returning_clause_references(PlannerInfo *root,
List *rlist,
Plan *topplan,
Index resultRelation);
extern void fix_opfuncids(Node *node);
extern void set_opfuncid(OpExpr *opexpr);
extern void set_sa_opfuncid(ScalarArrayOpExpr *opexpr);
extern void record_plan_function_dependency(PlannerGlobal *glob, Oid funcid);
extern void record_plan_function_dependency(PlannerInfo *root, Oid funcid);
extern void extract_query_dependencies(Node *query,
List **relationOids,
List **invalItems);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment