From: Masataka S. <pg...@gm...> - 2014-02-03 11:03:00
|
Hi, Ashutosh I made two patches in different approach. The borked plan is made from updating local_plan in the function before arriving pgxc_locate_grouping_columns. The patch #07 attached backups local_plan before it is re-written. Is this a right approach? As far as I'm concerned using PG_TRY to handle a non-error situation is dirty. I also wrote patch #06 in another approach to handle this case. It makes different alias which have the expression to have single sortgroupref. Do you have any concern about this approach? Regards. On 17 January 2014 16:47, Masataka Saito <pg...@gm...> wrote: > I would take option #1 if possible, but I think estimating impact of the > fix and testing requires intimate knowledge of SQL and the optimizer: How > the planner optimize a plan tree and what SQL could have two entries in a > target list with the same value, How ressortgroup, resno, plan's target > list and remote query's target list and base tlist is used by the > optimizer and executor. > It is still hard for me. > > My concern with 2nd option is how's the performance compared to the > shipped case and how much case are there that SQLs is not optimized. > I made some aggregation queries to test how's the query optimized with the > attached patch, and I found that just returning false in > pgxc_locate_grouping_columns() is not enough to fix this problem. > > For example, the next query generates a borked plan. > postgres=# CREATE TABLE tbl AS SELECT * FROM (SELECT generate_series(1,3) > as i) a, (SELECT generate_series(1,3) AS j) b; > INSERT 0 9 > postgres=# EXPLAIN VERBOSE SELECT i AS a, i AS b, avg(j) AS c FROM tblGROUP BY > a ORDER BY b; > QUERY PLAN > > > --------------------------------------------------------------------------------------------------- > GroupAggregate (cost=49.83..57.45 rows=10 width=8) > Output: tbl.i, tbl.i, avg(tbl.i) > -> Sort (cost=49.83..52.33 rows=1000 width=8) > Output: tbl.i, tbl.i > Sort Key: tbl.i > -> Data Node Scan on tbl "_REMOTE_TABLE_QUERY_" (cost=0.00..0.00 > rows=1000 width=8) > Output: tbl.i, tbl.i, (avg(tbl.j)) > Node/s: datanode1, datanode2 > Remote query: SELECT i, pg_catalog.int8_avg(avg(j)) FROM > ONLY public.tbl WHERE true > (9 rows) > > Of course, it fails. > postgres=# SELECT i AS a, i AS b, avg(j) AS c FROM tbl GROUP BY a ORDER > BY b; > ERROR: column "tbl.i" must appear in the GROUP BY clause or be used in > an aggregate function > > In any case, this problem needs more study. > > Regards. > > > > On Wed, Jan 15, 2014 at 7:18 PM, Ashutosh Bapat < > ash...@en...> wrote: > >> >> >> >> On Wed, Jan 15, 2014 at 1:55 PM, Masataka Saito <pg...@gm...> wrote: >> >>> Hi Ashutosh, >>> >>> Specified groupColIdx doesn't found in tlist. >>> >>> It seems that groupColIdx is correct and tlist is wrong. >>> Because the value of ressortgroupref in the target entry(base_tlist) >>> they want to locate was zero. >>> >>> The mechanism why it comes to be zero is that when >>> pgxc_rqplan_adjust_tlist updates base_tlist using >>> pgxc_build_shippable_tlist, a ressortgroupref in a target entry is >>> reset if a plan's target list has two equal Vars. >>> >>> >> You have spotted the problem correctly. While writing this code, I >> thought that having two entries in target list with same expression would >> be problem when it comes to setting references (set_plan_ref), and hence >> chose to disallow that. The assumption there was, if >> locate_grouping_columns() doesn't find the column, it would return an >> invalid value, and thus we can choose not to optimize that case. But >> locate_grouping_columns() is actually throwing error, which breaks the >> assumption. >> >> So, there are two possibilities >> 1. bite the bullet and let there be two entries in the targetlist with >> same expressions but different ressortgroupref and see if that approach has >> any other problems - we will have to test a bunch of scenarios to make sure >> that there is no problem there. >> 2. Modify pgxc_locate_grouping_columns() to return NULL when no >> expression is found with matching ressortgroupref. But that might need some >> magic with PG_TRY() and CATCH or code duplication. >> >> >>> It occurs when SELECT target has two aliases to the same column and each >>> are referred by GROUP BY clause and ORDER BY clause. I will show a >>> sample query at next. >>> >>> postgres-# CREATE TABLE tbl(i INT); >>> postgres-# SELECT i AS a, i AS b FROM tbl GROUP BY a ORDER BY b; >>> >>> It needs more analysis, but I'm thinking how about using >>> locate_grouping_columns with plan's target list and base_tlist instead >>> of pgxc_locate_grouping_columns. >>> >>> Regards. >>> >>> >>> On Thu, Nov 21, 2013 at 1:40 PM, Ashutosh Bapat < >>> ash...@en...> wrote: >>> >>>> Hi Masataka, >>>> To start debugging, you might want to fire this query with debugger >>>> breaking on errstart() or errfinish() or some error reporting hook where >>>> you can break. The stack stress should give you a lead as to where the >>>> error is coming from. It's high chance that it's coming from >>>> pgxc_locate_grouping_columns(). But anyway, start looking around the >>>> callers of get_sortgroupref_tle() and check their arguments. Best luck. >>>> >>>> >>>> On Thu, Nov 21, 2013 at 6:51 AM, Masataka Saito <pg...@gm...>wrote: >>>> >>>>> Thanks for your information. >>>>> It seems very helpful. >>>>> >>>>> cx=# explain verbose SELECT DISTINCT t2.b FROM t1 JOIN t2 ON t1.id = >>>>> t2.id GROUP BY b; >>>>> ERROR: XX000: ORDER/GROUP BY expression not found in targetlist >>>>> LOCATION: get_sortgroupref_tle, tlist.c:251 >>>>> >>>>> On Thu, Nov 21, 2013 at 9:53 AM, 鈴木 幸市 <ko...@in...> >>>>> wrote: >>>>> > Yes, you can do it with SET command as well just for a target >>>>> statement. >>>>> > >>>>> > Regards; >>>>> > --- >>>>> > Koichi Suzuki >>>>> > >>>>> > 2013/11/21 7:46、Michael Paquier <mic...@gm...> のメール: >>>>> > >>>>> >> On Thu, Nov 21, 2013 at 12:32 AM, Masataka Saito <pg...@gm...> >>>>> wrote: >>>>> >>> XC can't build a plan. >>>>> >>> >>>>> >>> db=# explain verbose SELECT DISTINCT t2.b FROM t1 JOIN t2 ON t1.id= >>>>> >>> t2.id GROUP BY b; >>>>> >>> ERROR: ORDER/GROUP BY expression not found in targetlist >>>>> >> This is a higher-level bug, planner bug just by looking at this code >>>>> >> path src/backend/optimizer/util/tlist.c... >>>>> >> >>>>> >> Note: Setting up VERBOSITY to verbose in .psqlrc helps grabbing more >>>>> >> details about the errors that occurred in server like the file name >>>>> + >>>>> >> name of this elog/ereport ERROR: >>>>> >> \set VERBOSITY verbose >>>>> >> For dev purposes it is a pretty useful default ;) >>>>> >> >>>>> >> Regards, >>>>> >> -- >>>>> >> Michael >>>>> >> >>>>> >> >>>>> ------------------------------------------------------------------------------ >>>>> >> Shape the Mobile Experience: Free Subscription >>>>> >> Software experts and developers: Be at the forefront of tech >>>>> innovation. >>>>> >> Intel(R) Software Adrenaline delivers strategic insight and >>>>> game-changing >>>>> >> conversations that shape the rapidly evolving mobile landscape. >>>>> Sign up now. >>>>> >> >>>>> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk >>>>> >> _______________________________________________ >>>>> >> Postgres-xc-developers mailing list >>>>> >> Pos...@li... >>>>> >> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers >>>>> >> >>>>> > >>>>> >>>>> >>>>> ------------------------------------------------------------------------------ >>>>> Shape the Mobile Experience: Free Subscription >>>>> Software experts and developers: Be at the forefront of tech >>>>> innovation. >>>>> Intel(R) Software Adrenaline delivers strategic insight and >>>>> game-changing >>>>> conversations that shape the rapidly evolving mobile landscape. Sign >>>>> up now. >>>>> >>>>> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk >>>>> _______________________________________________ >>>>> Postgres-xc-developers mailing list >>>>> Pos...@li... >>>>> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers >>>>> >>>> >>>> >>>> >>>> -- >>>> Best Wishes, >>>> Ashutosh Bapat >>>> EnterpriseDB Corporation >>>> The Postgres Database Company >>>> >>> >>> >> >> >> -- >> Best Wishes, >> Ashutosh Bapat >> EnterpriseDB Corporation >> The Postgres Database Company >> > > |