From: Ashutosh B. <ash...@en...> - 2013-09-26 08:22:26
|
On Thu, Sep 26, 2013 at 1:29 PM, Tomonari Katsumata < kat...@po...> wrote: > Hi Ashtosh, > > Thanks for your review! > > Previous patch(revised_fqs_r3.patch in [2013/08/14 19:55]) > doesn't use a shell script. > Instead, It used test_execute_direct_all_xc_**nodes() function. > > I've got a comment "don't use functions" from you, > but I could not come up with good idea for the testcase. > The testcase needs to get all node_names to "EXECUTE DIRECT" > against all nodes. > > I never said "don't use functions". Here's what I mailed "In the test file, isn't there any way, you can add the offending statent without any wrapper function test_execute_direct_all_xc_ nodes()? We should minimize the test-code to use only the minimal set of features. Since this statement no more gets into infinite loop, please do not use the statement timeout." > What is better for this testcase? > > May be we should just drop the testcase. Anyway, EXEC DIRECT is not production feature, but debug feature and whosoever wrote this feature has not added any testcases for it anyway. If anyone else has objections to dropping the testcase, please suggest Tomonari, a better to write it. > regards, > > ---------------- > NTT Software Corporation > Tomonari Katsumata > > (2013/09/26 15:44), Ashutosh Bapat wrote: > > Hi Tomonary, > > The testcase you have written uses a shell script. This is not the > standard > > way to add testcases. Can you please see if the testcase can be added > > directly as SQL? Please send a revised patch ASAP, so that it can be > > committed. I would have liked to see more testcases for EXEC DIRECT, but > > since it's a debug only feature (not for production), it gets lesser > > priority in testing and thus we do not see testcases for it. > > > > > > On Thu, Sep 26, 2013 at 8:26 AM, Tomonari Katsumata < > > kat...@po....**jp <kat...@po...>> > wrote: > > > >> Hi, > >> > >> Does anyone check these patches? > >> If there are no comments, please commit it. > >> > >> regards, > >> > >> ---------------------- > >> NTT Software Corporation > >> Tomonari Katsumata > >> > >> -------- Original Message -------- > >> Subject: Re: [Postgres-xc-developers] Query Planning bug ? > >> Date: Thu, 5 Sep 2013 21:34:32 +0900 > >> From: Tomonari Katsumata <t.k...@gm...> > >> To: Tomonari Katsumata <katsumata.tomonari@po.ntts.****co.jp<http://co.jp> > <katsumata.tomonari@po.**ntts.co.jp <kat...@po...>> > >>> > >> CC: Ashutosh Bapat <ashutosh.bapat@enterprisedb.****com< > ashutosh.bapat@**enterprisedb.com <ash...@en...>>>, > >> pgxc-hackers mailing list <postgres-xc-developers@lists.**** > sourceforge.net<postgres-xc-**dev...@li...urceforge.**net<pos...@li...> > > > > >>> > >> > >> > >> Hi, > >> > >> I'm sorry for long long leaving this thread alone. > >> I've devided the patch to source part and regression part. > >> (against f8e0f994bff1faefa9b1fc2f6e584a****1d3de70241) > > >> > >> I did not change any process on source part. > >> (revised_fqs_r4.patch) > >> > >> I added a script into src/test/regress/sql directory for > >> changing regression part. > >> (add_test_xc_misc.patch and execute_direct_regress.sh) > >> When regression test runs, the script is executed instead of > >> user defined function. > >> > >> Please check and commit it. > >> > >> regards, > >> ------------------------ > >> NTT Software Corporation > >> Tomonari Katsumata > >> > >> > >> 2013/8/14 Tomonari Katsumata <katsumata.tomonari@po.ntts.****co.jp<http://co.jp> > <katsumata.tomonari@po.**ntts.co.jp <kat...@po...>> > > >>> > >> > >>> Hi Ashtosh, > >>> > >>> Thanks for your comment! > >>> > >>> > >>>> The code changes are fine, but, the comment in > >> pgxc_collect_RTE_walker() > >>>> seems to specific. I think it should read, "create a copy of query's > >>> range > >>>> table, so that it can be linked with other RTEs in the collector's > >>> context." > >>>> > >>> I've revised the comment as you suggested. > >>> > >>> > >>>> In the test file, isn't there any way, you can add the offending > >> statent > >>>> without any wrapper function test_execute_direct_all_xc_***** > *nodes()? > > >> We > >> > >>> should > >>>> minimize the test-code to use only the minimal set of features. Since > >>> this > >>>> statement no more gets into infinite loop, please do not use the > >>> statement > >>>> timeout. > >>> > >>> I've gotten rid of using statement timeout. > >>> But I couldn't come up any idea to testing more simply, > >>> so the function is remaining as same as before patch. > >>> Any ideas? > >>> > >>> Here is the new patch. > >>> (against dec40008b3d689911566514614c511******1c0a61327d) > > >> > >>> > >>> > >>> regards, > >>> ------------- > >>> NTT Software Corporation > >>> Tomonari Katsumata > >>> > >>> > >> > >> > >> > >> > >> ------------------------------**------------------------------** > ------------------ > >> October Webinars: Code for Performance > >> Free Intel webinars can help you accelerate application performance. > >> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most > >> from > >> the latest Intel processors and coprocessors. See abstracts and > register > > >> http://pubads.g.doubleclick.**net/gampad/clk?id=60133471&iu=** > /4140/ostg.clktrk<http://pubads.g.doubleclick.net/gampad/clk?id=60133471&iu=/4140/ostg.clktrk> > >> ______________________________**_________________ > >> Postgres-xc-developers mailing list > >> Postgres-xc-developers@lists.**sourceforge.net<Pos...@li...> > >> https://lists.sourceforge.net/**lists/listinfo/postgres-xc-**developers<https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers> > >> > >> > > > > > > > -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company |