Re: [HACKERS] Partition-wise aggregation/grouping

2017-11-01 Thread Jeevan Chalke
On Sat, Oct 28, 2017 at 3:07 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Fri, Oct 27, 2017 at 1:01 PM, Jeevan Chalke > <jeevan.cha...@enterprisedb.com> wrote: > > 1. Added separate patch for costing Append node as discussed up-front in > the > > patch-set

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-27 Thread Jeevan Chalke
patch. 3. Updated rows in test-cases so that we will get partition-wise plans. Thanks On Wed, Oct 18, 2017 at 9:53 AM, Dilip Kumar <dilipbal...@gmail.com> wrote: > On Tue, Oct 17, 2017 at 10:44 PM, Jeevan Chalke > <jeevan.cha...@enterprisedb.com> wrote: > > > >

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-17 Thread Jeevan Chalke
On Tue, Oct 17, 2017 at 7:13 PM, Dilip Kumar <dilipbal...@gmail.com> wrote: > On Fri, Oct 13, 2017 at 12:06 PM, Jeevan Chalke > <jeevan.cha...@enterprisedb.com> wrote: > > > While playing around with the patch I have noticed one regression with > the partial part

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-16 Thread Jeevan Chalke
umber. > Agree, but those magic numbers used only once at that place. But here there are two places. So if someone wants to update it, (s)he needs to make sure to update that at two places. To minimize that risk, having a #define seems better. > > -- > David Rowley

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-13 Thread Jeevan Chalke
131.400) - 16925.01) / 100); 1.8 -- With 1 rows (so no Gather too) # select current_Setting('cpu_tuple_cost')::float8 / ((170.01 * (1.919 / 1.424) - 170.01) / 1); 1.7 So it is not so straight forward to come up the correct heuristic here. Thus using 50% of cpu_tuple_cost look good to

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-10 Thread Jeevan Chalke
On Tue, Oct 10, 2017 at 10:27 AM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > On Tue, Oct 10, 2017 at 3:15 AM, David Rowley > <david.row...@2ndquadrant.com> wrote: > > On 10 October 2017 at 01:10, Jeevan Chalke > > <jeevan.cha...@enterprisedb.com>

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-09 Thread Jeevan Chalke
alization path seems to be similar > to the > code that adds finalization path for parallel query. May be we could take > out > common code into a function and call that function in two places. I see > this > function as accepting a partial aggregation/grouping path and returni

Re: [HACKERS] Partition-wise aggregation/grouping

2017-09-27 Thread Jeevan Chalke
orthwhile to fix > the reason why we would require this GUC. If the regular aggregation > has cost lesser than partition-wise aggregation in most of the cases, > then probably we need to fix the cost model. > Yep. I will have a look mean-while. > > I will continue reviewing res

Re: [HACKERS] Partition-wise aggregation/grouping

2017-09-22 Thread Jeevan Chalke
se steps on psql reproduces the crash (not consistent though). Looking into it. Thanks for reporting. > Thanks & Regards, > Rajkumar Raghuwanshi > QMG, EnterpriseDB Corporation > -- Jeevan Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company

Re: [HACKERS] Partition-wise join for join between (declaratively) partitioned tables

2017-09-20 Thread Jeevan Chalke
ble use of partition-wise strategy, one for each of join, > aggregation and sorting. Having granular switches would be useful for > debugging and may be to turn partition-wise strategies off when they > are not optimal. I think having a granular control over each of these optimization wi

Re: [HACKERS] Partition-wise aggregation/grouping

2017-09-18 Thread Jeevan Chalke
On Tue, Sep 12, 2017 at 6:21 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > > > On Tue, Sep 12, 2017 at 3:24 PM, Rajkumar Raghuwanshi < > rajkumar.raghuwan...@enterprisedb.com> wrote: > >> >> Hi Jeevan, >> >> I have started testi

Re: [HACKERS] Constraint exclusion for partitioned tables

2017-09-13 Thread Jeevan Chalke
On Tue, Sep 12, 2017 at 8:12 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Tue, Sep 12, 2017 at 7:08 AM, Jeevan Chalke > <jeevan.cha...@enterprisedb.com> wrote: > > This patch clearly improves the planning time with given conditions. > > > > To veri

Re: [HACKERS] Partition-wise aggregation/grouping

2017-09-12 Thread Jeevan Chalke
On Tue, Sep 12, 2017 at 3:24 PM, Rajkumar Raghuwanshi < rajkumar.raghuwan...@enterprisedb.com> wrote: > > On Fri, Sep 8, 2017 at 5:47 PM, Jeevan Chalke <jeevan.chalke@enterprisedb. > com> wrote: > >> Here are the new patch-set re-based on HEAD (f0a0c17) and >

Re: [HACKERS] Constraint exclusion for partitioned tables

2017-09-12 Thread Jeevan Chalke
ble has no > storage of its own. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > -- Jeevan Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

2017-09-11 Thread Jeevan Chalke
Hi Pavel, On Sat, Sep 9, 2017 at 11:42 AM, Pavel Stehule <pavel.steh...@gmail.com> wrote: > Hi > > 2017-09-08 9:36 GMT+02:00 Jeevan Chalke <jeevan.cha...@enterprisedb.com>: > >> Hi Pavel, >> I like the idea of using parameter name instead of $n symbols.

Re: [HACKERS] Partition-wise aggregation/grouping

2017-09-08 Thread Jeevan Chalke
On Wed, Aug 23, 2017 at 4:43 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > Hi, > > Attached is the patch to implement partition-wise aggregation/grouping. > > As explained earlier, we produce a full aggregation for each partition when > partition keys are

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

2017-09-08 Thread Jeevan Chalke
t, then use that else use $n name. +*/ + argvariable = plpgsql_build_variable((argnames && argnames[i][0] != '\0') ? +argnames[i] : buf, +0, argdtype, false);

Re: [HACKERS] WIP: Aggregation push-down

2017-09-07 Thread Jeevan Chalke
> > -- > Antonin Houska > Cybertec Schönig & Schönig GmbH > Gröhrmühlgasse 26 > A-2700 Wiener Neustadt > Web: http://www.postgresql-support.de, http://www.cybertec.at > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > -- Jeevan Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company

Re: [HACKERS] Replacing lfirst() with lfirst_node() appropriately in planner.c

2017-09-05 Thread Jeevan Chalke
finitely; please add to the open commitfest. > > Thanks. Added. https://commitfest.postgresql.org/14/1195/ > -- > Best Wishes, > Ashutosh Bapat > EnterpriseDB Corporation > The Postgres Database Company > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@p

Re: [HACKERS] Partition-wise aggregation/grouping

2017-08-23 Thread Jeevan Chalke
...@mail.gmail.com On Tue, Mar 21, 2017 at 12:47 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > Hi all, > > Declarative partitioning is supported in PostgreSQL 10 and work is already > in > progress to support partition-wise joins. Here is a proposal for > partition-wis

[HACKERS] Server crash (FailedAssertion) due to catcache refcount mis-handling

2017-08-08 Thread Jeevan Chalke
e_backend(); I thought it worth posting here to get others attention. I have observed this on the master branch, but can also be reproducible on back-branches. Thanks -- Jeevan Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company Pr

Re: [HACKERS] Partition-wise aggregation/grouping

2017-04-27 Thread Jeevan Chalke
get apply. Can you please provide the HEAD and any other changes required to be applied first? How the plan look like when GROUP BY key does not match with the partitioning key i.e. GROUP BY b.v ? > > [1] https://www.postgresql.org/message-id/9666.1491295317%40localhost > > [2] h

Re: [HACKERS] Partition-wise aggregation/grouping

2017-03-23 Thread Jeevan Chalke
On Tue, Mar 21, 2017 at 1:47 PM, Antonin Houska <a...@cybertec.at> wrote: > Jeevan Chalke <jeevan.cha...@enterprisedb.com> wrote: > > > Declarative partitioning is supported in PostgreSQL 10 and work is > already in > > progress to support partition-wise joins. H

[HACKERS] Partition-wise aggregation/grouping

2017-03-21 Thread Jeevan Chalke
join feature. [1] https://www.postgresql.org/message-id/CAFjFpRcbY2QN3cfeMTzVEoyF5Lfku-ijyNR%3DPbXj1e%3D9a%3DqMoQ%40mail.gmail.com Thanks -- Jeevan Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company pg_partwise_agg_WIP.patch

Re: [HACKERS] PassDownLimitBound for ForeignScan/CustomScan [take-2]

2017-03-13 Thread Jeevan Chalke
OIN is pushed down to remote server, thus need to update this comment. Rest of the changes look good to me. Thanks -- Jeevan Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company

[HACKERS] PassDownLimitBound for ForeignScan/CustomScan [take-2]

2016-11-24 Thread Jeevan Chalke
r aggregate functions. 3. Typo: don's => don't Rest of the changes look good to me. Thanks > Thanks, > > PG-Strom Project / NEC OSS Promotion Center > KaiGai Kohei <kai...@ak.jp.nec.com> > > > > -Original Message- > > From: Robert Haas [mailto

Re: [HACKERS] PassDownLimitBound for ForeignScan/CustomScan [take-2]

2016-11-08 Thread Jeevan Chalke
nk this is not possible here since 0 can be a legal user provided value which cannot be set as a default (default is all rows). However do you think, can we avoid that? Is there any other way so that we don't need every node having ps_numTuples to be set explicitly? Apart from this patch look

Re: [HACKERS] Substantial bloat in postgres_fdw regression test runtime

2016-11-03 Thread Jeevan Chalke
On Wed, Nov 2, 2016 at 10:09 PM, Tom Lane wrote: > In 9.6, "make installcheck" in contrib/postgres_fdw takes a shade > under 3 seconds on my machine. In HEAD, it's taking 10 seconds. > I am not happy, especially not since there's no parallelization > of the contrib

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-24 Thread Jeevan Chalke
On Sat, Oct 22, 2016 at 9:09 PM, Tom Lane wrote: > brolga is still not terribly happy with this patch: it's choosing not to > push down the aggregates in one of the queries. While I failed to > duplicate that result locally, investigation suggests that brolga's result > is

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-20 Thread Jeevan Chalke
On Thu, Oct 20, 2016 at 12:49 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > The patch compiles and make check-world doesn't show any failures. > > >> > > > > > > I have tried it. Attached separate patch for it. > > However I have noticed that istoplevel is always false (at-least

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-12 Thread Jeevan Chalke
On Thu, Sep 8, 2016 at 10:41 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > > > I think we should try to measure performance gain because of aggregate > pushdown. The EXPLAIN > doesn't show actual improvement in the execution times. > I did performance testing for aggregate push

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-30 Thread Jeevan Chalke
On Mon, Sep 26, 2016 at 6:15 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > This patch will need some changes to conversion_error_callback(). That > function reports an error in case there was an error converting the > result obtained from the foreign server into an internal datum

Re: [HACKERS] Add support for restrictive RLS policies

2016-09-29 Thread Jeevan Chalke
Hi Stephen, > 4. It will be good if we have an example for this in section > > "5.7. Row Security Policies" > > I haven't added one yet, but will plan to do so. > > I think you are going to add this in this patch itself, right? I have reviewed your latest patch and it fixes almost all my review

Re: [HACKERS] Add support for restrictive RLS policies

2016-09-27 Thread Jeevan Chalke
On Tue, Sep 27, 2016 at 12:45 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > Hello Stephen, > > I am reviewing the latest patch in detail now and will post my review > comments later. > Here are the review comments: 1. In documentation, we should put bo

Re: [HACKERS] Add support for restrictive RLS policies

2016-09-27 Thread Jeevan Chalke
Hello Stephen, On Tue, Sep 27, 2016 at 12:57 AM, Stephen Frost <sfr...@snowman.net> wrote: > Jeevan, > > * Jeevan Chalke (jeevan.cha...@enterprisedb.com) wrote: > > I have started reviewing this patch and here are couple of points I have > > observed so far: >

Re: [HACKERS] Add support for restrictive RLS policies

2016-09-26 Thread Jeevan Chalke
Hi, I have started reviewing this patch and here are couple of points I have observed so far: 1. Patch applies cleanly 2. make / make install / initdb all good. 3. make check (regression) FAILED. (Attached diff file for reference). Please have a look over failures. Meanwhile I will go ahead

Re: [HACKERS] Add support for restrictive RLS policies

2016-09-26 Thread Jeevan Chalke
On Mon, Sep 12, 2016 at 7:27 AM, Stephen Frost wrote: > Robert, > > * Robert Haas (robertmh...@gmail.com) wrote: > > On Thu, Sep 8, 2016 at 5:21 PM, Tom Lane wrote: > > > Stephen Frost writes: > > >> * Alvaro Herrera

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-16 Thread Jeevan Chalke
On Mon, Sep 12, 2016 at 5:19 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > > > On Mon, Sep 12, 2016 at 12:20 PM, Prabhat Sahu < > prabhat.s...@enterprisedb.com> wrote: > >> Hi, >> >> While testing "Aggregate pushdown", i found

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-12 Thread Jeevan Chalke
On Mon, Sep 12, 2016 at 12:20 PM, Prabhat Sahu < prabhat.s...@enterprisedb.com> wrote: > Hi, > > While testing "Aggregate pushdown", i found the below error: > -- GROUP BY alias showing different behavior after adding patch. > > -- Create table "t1", insert few records. > create table t1(c1 int);

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-12 Thread Jeevan Chalke
On Thu, Sep 8, 2016 at 10:41 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > > > > >> While checking for shippability, we build the target list which is passed >> to >> the foreign server as fdw_scan_tlist. The target list contains >> a. All the GROUP BY expressions >> b. Shippable

Re: [HACKERS] PassDownLimitBound for ForeignScan/CustomScan

2016-09-06 Thread Jeevan Chalke
Hi, Changes look good to me. However there are couple of minor issues need to be fixed. 1. "under" repeated on second line. Please remove. +if and when CustomScanState is located under +under LimitState; which implies the underlying node is not 2. Typo: dicsussion => discussion Please

Re: [HACKERS] PassDownLimitBound for ForeignScan/CustomScan

2016-09-02 Thread Jeevan Chalke
Hi, On Mon, Aug 29, 2016 at 7:25 AM, Kouhei Kaigai wrote: > Hello, > > The attached patch adds an optional callback to support special > optimization > if ForeignScan/CustomScan are located under the Limit node in plan-tree. > > Our sort node wisely switches the behavior

Re: [HACKERS] Small patch for snapmgr.c

2016-08-31 Thread Jeevan Chalke
Hi Aleksander, This has already been fixed with commit 4f9f495889d3d410195c9891b58228727b340189 Thanks On Fri, Apr 8, 2016 at 6:02 PM, Aleksander Alekseev < a.aleks...@postgrespro.ru> wrote: > Hello > > Currently there is a following piece of code in snapmgr.c: > > ``` > /* Copy all required

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-08-31 Thread Jeevan Chalke
On Tue, Aug 30, 2016 at 6:51 PM, Pavel Stehule <pavel.steh...@gmail.com> wrote: > Hi > > 2016-08-30 15:02 GMT+02:00 Jeevan Chalke <jeevan.cha...@enterprisedb.com>: > >> Hi all, >> >> Attached is the patch which adds support to push down aggregatio

[HACKERS] Strange result with LATERAL query

2016-08-23 Thread Jeevan Chalke
Hi, While playing with LATERAL along with some aggregates in sub-query, I have observed somewhat unusual behavior. Consider following steps: create table tab1(c1 int, c2 int); insert into tab1 select id, 1 from generate_series(1, 3) id; create function sum_tab1(extra int) returns setof bigint

Re: [HACKERS] postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

2016-02-09 Thread Jeevan Chalke
Hi, I have reviewed the patch and it looks good to me. make/make install/make check is fine (when done without -Wall -Werror). Here are few comments: 1. With -Wall -Werror, I see couple of warnings: postgres_fdw.c: In function ‘estimate_path_cost_size’: postgres_fdw.c:2248:13: error: ‘run_cost’

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

2015-10-16 Thread Jeevan Chalke
On Thu, Oct 15, 2015 at 10:44 PM, Robert Haas wrote: > On Thu, Oct 15, 2015 at 3:04 AM, Kyotaro HORIGUCHI > wrote: > > I confirmed that an epqtuple of foreign parameterized scan is > > correctly rejected by fdw_recheck_quals with modified

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

2015-10-16 Thread Jeevan Chalke
On Fri, Oct 16, 2015 at 3:10 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > > > On Thu, Oct 15, 2015 at 10:44 PM, Robert Haas <robertmh...@gmail.com> > wrote: > >> On Thu, Oct 15, 2015 at 3:04 AM, Kyotaro HORIGUCHI >> <horiguchi.kyot

Re: [HACKERS] Getting sorted data from foreign server

2015-10-13 Thread Jeevan Chalke
> On Thu, Oct 8, 2015 at 9:39 PM, Robert Haas wrote: > >> >> In the interest of full disclosure, I asked Ashutosh to work on this >> patch and have discussed the design with him several times. I believe >> that this is a good direction for PostgreSQL to be going. It's >>

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

2015-10-09 Thread Jeevan Chalke
On Fri, Oct 9, 2015 at 3:35 PM, Etsuro Fujita wrote: Hi, Just to have hands on, I started looking into this issue and trying to grasp it as this is totally new code for me. And later I want to review this code changes. I have noticed that, this thread started

Re: [HACKERS] TEXT vs VARCHAR join qual push down diffrence, bug or expected?

2015-09-24 Thread Jeevan Chalke
On Wed, Sep 23, 2015 at 7:29 PM, Tom Lane wrote: > Removing that entirely would be quite incorrect, because then you'd be > lying to the parent node about what collation your node outputs. > Yes. I too thought so and thus wanted to fix that code block by considering the

Re: [HACKERS] TEXT vs VARCHAR join qual push down diffrence, bug or expected?

2015-09-24 Thread Jeevan Chalke
On Wed, Sep 23, 2015 at 10:15 PM, Tom Lane wrote: > I wrote: > > Hm ... actually, we probably need *both* types of changes if that's > > what we believe the state values mean. > > I too was confused with the state explanations from the code-comments which we have them now.

Re: [HACKERS] TEXT vs VARCHAR join qual push down diffrence, bug or expected?

2015-09-24 Thread Jeevan Chalke
On Thu, Sep 24, 2015 at 10:22 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Jeevan Chalke <jeevan.cha...@enterprisedb.com> writes: > > On Wed, Sep 23, 2015 at 10:15 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > >> After a bit more thinking and experimentation, I prop

Re: [HACKERS] TEXT vs VARCHAR join qual push down diffrence, bug or expected?

2015-09-23 Thread Jeevan Chalke
Hi Tom, On Tue, Sep 22, 2015 at 12:31 AM, Tom Lane wrote: > > I think you're blaming the wrong code; RelabelType is handled basically > the same as most other cases. > > It strikes me that this function is really going about things the wrong > way. Rather than trying to

[HACKERS] TEXT vs VARCHAR join qual push down diffrence, bug or expected?

2015-09-21 Thread Jeevan Chalke
Hi, It is observed that, when we have one remote (huge) table and one local (small) table and a join between them, then 1. If the column type is text, then we push the join qual to the remote server, so that we will have less rows to fetch, and thus execution time is very less. 2. If

Re: [HACKERS] GSets: Fix bug involving GROUPING and HAVING together

2015-07-24 Thread Jeevan Chalke
Hi, This will fail too. Note that, when we have only one element in GROUPING SETS, we add that in group by list and set parse-groupingSets to NULL. And hence it will have same issue. However tests added in my patch failing too. Thanks -- Jeevan B Chalke Principal Software Engineer, Product

[HACKERS] Gsets: ROW expression semantic broken between 9.4 and 9.5

2015-07-22 Thread Jeevan Chalke
Hi It looks like we have broken the ROW expression without explicit ROW keyword in GROUP BY. I mean, after Grouping sets merge, if we have (c1, c2) in group by, we are treating it as ROW expression for grouping, but at the same time we are allowing individual column in the target list. However

Re: [HACKERS] Grouping Sets: Fix unrecognized node type bug

2015-07-20 Thread Jeevan Chalke
On Sat, Jul 18, 2015 at 12:27 AM, Andrew Gierth and...@tao11.riddles.org.uk wrote: Kyotaro == Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes: Kyotaro Hello, this looks to be a kind of thinko. The attached patch Kyotaro fixes it. No, that's still wrong. Just knowing that there

[HACKERS] GSets: Getting collation related error when GSets has text column

2015-07-17 Thread Jeevan Chalke
Hi, When we have text column in the GROUPING SETS (and with some specific order of columns), we are getting error saying could not determine which collation to use for string comparison Here is the example: postgres=# select sum(ten) from onek group by rollup(four::text), two order by 1; ERROR:

Re: [HACKERS] Grouping Sets: Fix unrecognized node type bug

2015-07-17 Thread Jeevan Chalke
On Wed, Jul 15, 2015 at 10:21 PM, Andrew Gierth and...@tao11.riddles.org.uk wrote: Jeevan == Jeevan Chalke jeevan.cha...@enterprisedb.com writes: Jeevan Hi, Jeevan It looks like we do support nested GROUPING SETS, I mean Sets Jeevan withing Sets, not other types. However this nesting

[HACKERS] Grouping Sets: Fix unrecognized node type bug

2015-07-15 Thread Jeevan Chalke
Hi, It looks like we do support nested GROUPING SETS, I mean Sets withing Sets, not other types. However this nesting is broken. Here is the simple example where I would expect three rows in the result. But unfortunately it is giving unrecognized node type error. Which is something weird and

[HACKERS] GSets: Fix bug involving GROUPING and HAVING together

2015-07-14 Thread Jeevan Chalke
Hi, I have observed some fishy behavior related to GROUPING in HAVING clause and when we have only one element in GROUPING SETS. Basically, when we have only one element in GROUING SETS, we are assuming it as a simple GROUP BY with one column. Due to which we are ending up with this error. If

Re: [HACKERS] GSets: Fix bug involving GROUPING and HAVING together

2015-07-14 Thread Jeevan Chalke
On Tue, Jul 14, 2015 at 4:23 PM, Andrew Gierth and...@tao11.riddles.org.uk wrote: Jeevan == Jeevan Chalke jeevan.cha...@enterprisedb.com writes: Jeevan Basically, when we have only one element in GROUING SETS, we Jeevan are assuming it as a simple GROUP BY with one column. Due to Jeevan

Re: [HACKERS] [PATCH] two-arg current_setting() with fallback

2015-07-07 Thread Jeevan Chalke
On Fri, Jul 3, 2015 at 2:12 AM, Tom Lane t...@sss.pgh.pa.us wrote: Jeevan Chalke jeevan.cha...@enterprisedb.com writes: Attached patch which fixes my review comments. Applied with minor adjustments (mostly cosmetic, but did neither of you notice the compiler warning?) Oops. Sorry

[HACKERS] Missing tab-complete for PASSWORD word in CREATE ROLE syntax

2015-06-19 Thread Jeevan Chalke
Hi, I have observed that we are not tab-completing word PASSWORD in the following syntaxes: 1. CREATE|ALTER ROLE|USER rolname 2. CREATE|ALTER ROLE|USER rolname WITH PASSWORD is used many times and should be in the tab-complete list. Was there any reason we have deliberately kept this out? If

[HACKERS] Dead code in Create/RenameRole() after RoleSpec changes related to CURRENT/SESSION_USER

2015-06-09 Thread Jeevan Chalke
Hi, I found some dead code in CREATE/RENAME ROLE code path. Attached patch to remove those. We have introduced RoleSpec and handled public and none role names in grammar itself. We do have these handling in CreateRole() and RenameRole() which is NO more valid now. Here is the related commit:

Re: [HACKERS] bugfix: incomplete implementation of errhidecontext

2015-06-09 Thread Jeevan Chalke
On Mon, Jun 8, 2015 at 8:19 PM, Andres Freund and...@anarazel.de wrote: On 2015-06-08 14:44:53 +, Jeevan Chalke wrote: The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec

Re: [HACKERS] psql :: support for \ev viewname and \sv viewname

2015-06-09 Thread Jeevan Chalke
Hi Patch looks excellent now. No issues. Found a typo which I have fixed in the attached patch. Thanks -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company diff --git a/doc/src/sgml/ref/psql-ref.sgml

Re: [HACKERS] psql :: support for \ev viewname and \sv viewname

2015-06-09 Thread Jeevan Chalke
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed Patch looks good to pass to committer. The new status of

Re: [HACKERS] bugfix: incomplete implementation of errhidecontext

2015-06-08 Thread Jeevan Chalke
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed This is trivial bug fix in the area of hiding error context.

Re: [HACKERS] [PATCH] two-arg current_setting() with fallback

2015-06-04 Thread Jeevan Chalke
Hi, Attached patch which fixes my review comments. Since code changes were good, just fixed reported cosmetic changes. David, can you please cross check? Thanks -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company diff

Re: [HACKERS] [PATCH] two-arg current_setting() with fallback

2015-06-04 Thread Jeevan Chalke
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed I have reviewed the patch. Here are my review comments: 1.

Re: [HACKERS] psql :: support for \ev viewname and \sv viewname

2015-06-01 Thread Jeevan Chalke
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, failed I have reviewed this patch. Most of the code is just

Re: [HACKERS] bugfix: incomplete implementation of errhidecontext

2015-05-29 Thread Jeevan Chalke
Pavel, will it be good if you separately submit the bugfix: incomplete implementation of errhidecontext patch in this commitfest? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

2015-05-29 Thread Jeevan Chalke
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: tested, passed Spec compliant: tested, passed Documentation:not tested I agree with Peter that We don't tab-complete everything we possibly

Re: [HACKERS] Add LINE: hint when schemaname.typename is a non-existent schema

2015-03-18 Thread Jeevan Chalke
Álvaro, I think, there are few open questions here and thus marking it back to Waiting on Author. Please have your views on the review comments already posted. Also make changes as Tom suggested about placing pstate at the beginning. I am more concerned about this: 1. postgres=# create or

Re: [HACKERS] How about to have relnamespace and relrole?

2015-03-12 Thread Jeevan Chalke
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed Looks good. Passing it to committer. The new status of this

Re: [HACKERS] How about to have relnamespace and relrole?

2015-03-04 Thread Jeevan Chalke
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed 1. +#include utils/acl.h Can you please

Re: [HACKERS] Add LINE: hint when schemaname.typename is a non-existent schema

2015-03-03 Thread Jeevan Chalke
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: tested, failed Spec compliant: not tested Documentation:not tested Tom suggested few changes already which I too think author needs to

Re: [HACKERS] How about to have relnamespace and relrole?

2015-02-27 Thread Jeevan Chalke
The attatched are the fourth version of this patch. 0001-Add-regrole_v4.patch 0002-Add-regnamespace_v4.patch Seems like you have missed to attach both the patches. -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company

Re: [HACKERS] Review of GetUserId() Usage

2015-02-26 Thread Jeevan Chalke
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed I have reviewed the patch. Patch is excellent in shape and

Re: [HACKERS] How about to have relnamespace and relrole?

2015-02-24 Thread Jeevan Chalke
Hi, Personally, I was looking for something like this as I need to see rolename and namespace name many times in my queries rather than it's oid. But making a JOIN expression every-time was a pain. This certainly makes it easier. And I see most DBAs are looking for it. I agree on Tom's concern

Re: [HACKERS] How about to have relnamespace and relrole?

2015-02-24 Thread Jeevan Chalke
Reviewed posted directly on mail thread instead of posting it on commitfest app. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] detect custom-format dumps in psql and emit a useful error

2014-10-17 Thread Jeevan Chalke
Hi, Regarding Loading Custom Format Dump: === When we supply plain sql file to pg_restore, we get following error: $ ./install/bin/pg_restore a.sql pg_restore: [archiver] input file does not appear to be a valid archive So I would expect similar kind of message when we provide non-plain sql file

Re: [HACKERS] Re: proposal: ignore null fields in not relation type composite type based constructors

2014-09-05 Thread Jeevan Chalke
On Thu, Sep 4, 2014 at 11:41 AM, Pavel Stehule pavel.steh...@gmail.com wrote: I am sory too much patches :) Patch looks good to me. Marking Ready for Committer. Thanks Regards Pavel -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The

Re: [HACKERS] Re: proposal: ignore null fields in not relation type composite type based constructors

2014-09-03 Thread Jeevan Chalke
Hi Pavel, Here are few more comments on new implementation. 1. /* - * SQL function row_to_json(row) + * SQL function row_to_json(row record, pretty bool, ignore_nulls bool) */ In above comments, parameter name row should changed to rowval. 2. -DATA(insert OID = 3155 ( row_to_json

Re: [HACKERS] Re: proposal: ignore null fields in not relation type composite type based constructors

2014-09-03 Thread Jeevan Chalke
Hi Pavel, You have attached wrong patch. Thanks -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company

Re: [HACKERS] Re: proposal: ignore null fields in not relation type composite type based constructors

2014-09-02 Thread Jeevan Chalke
Hi Pavel, it needs a redesign of original implementation, we should to change API to use default values with named parameters but it doesn't help too much (although it can be readable little bit more) instead row_to_json(x, false, true) be row_ro_json(x, ignore_null := true) it is not

Re: [HACKERS] Re: proposal: ignore null fields in not relation type composite type based constructors

2014-09-01 Thread Jeevan Chalke
Hi Pavel, Patch does look good to me. And found no issues as such. However here are my optional suggestions: 1. Frankly, I did not like name of the function row_to_json_pretty_choosy. Something like row_to_json_pretty_ignore_nulls seems better to me. 2. To use ignore nulls feature, I have to

Re: [HACKERS] Re: proposal: ignore null fields in not relation type composite type based constructors

2014-08-22 Thread Jeevan Chalke
Hi Pavel, You have said that XMLFOREST has something which ignores nulls, what's that? Will you please provide an example ? I am NOT sure, but here you are trying to omit entire field from the output when its value is NULL. But that will add an extra efforts at other end which is using output of

Re: [HACKERS] add line number as prompt option to psql

2014-08-22 Thread Jeevan Chalke
I would like to ignore this as UINTMAX lines are too much for a input buffer to hold. It is almost NIL chances to hit this. Yeah, most likely you will run out of memory before reaching that point, or out of patience. Yep. BTW, I have marked this as waiting for committer. Thanks --

Re: [HACKERS] add line number as prompt option to psql

2014-08-20 Thread Jeevan Chalke
Hi, I have reviewed this: I have initialize cur_lineno to UINTMAX - 2. And then observed following behaviour to check wrap-around. postgres=# \set PROMPT1 '%/[%l]%R%# ' postgres[18446744073709551613]=# \set PROMPT2 '%/[%l]%R%# ' postgres[18446744073709551613]=# select

Re: [HACKERS] add line number as prompt option to psql

2014-07-11 Thread Jeevan Chalke
Hi, Found new issues with latest patch: Thank you for reviewing the patch with variable cases. I have revised the patch, and attached latest patch. A: Will you please explain the idea behind these changes ? I thought wrong about adding new to tail of query_buf. The latest patch does

Re: [HACKERS] add line number as prompt option to psql

2014-07-11 Thread Jeevan Chalke
Hi, On Fri, Jul 11, 2014 at 3:13 PM, Sawada Masahiko sawada.m...@gmail.com wrote: To my understating cleanly, you means that line number is not changed when newline has reached to INT_MAX, is incorrect? As per my thinking yes. And the line number should be switched to 1 when line

Re: [HACKERS] add line number as prompt option to psql

2014-07-10 Thread Jeevan Chalke
Hi, Found few more bugs in new code: A: This got bad: jeevan@ubuntu:~/pg_master$ ./install/bin/psql postgres psql (9.5devel) Type help for help. postgres=# \set PROMPT1 '%/[%l]%R%# ' postgres[1]=# \set PROMPT2 '%/[%l]%R%# ' postgres[1]=# select postgres[2]-# * postgres[3]-# from postgres[4]-#

Re: [HACKERS] Allowing NOT IN to use ANTI joins

2014-07-09 Thread Jeevan Chalke
Hi, With further testing I noticed that the patch was not allowing ANTI joins in cases like this: explain select * from a where id not in(select x from b natural join c); I too found this with natural joins and was about to report that. But its good that you found that and fixed it as

Re: [HACKERS] add line number as prompt option to psql

2014-07-07 Thread Jeevan Chalke
at 7:17 PM, Jeevan Chalke jeevan.cha...@enterprisedb.com wrote: Hi Sawada Masahiko, I liked this feature. So I have reviewed it. Changes are straight forward and looks perfect. No issues found with make/make install/initdb/regression. However I would suggest removing un-necessary

Re: [HACKERS] Allowing NOT IN to use ANTI joins

2014-07-02 Thread Jeevan Chalke
On Sun, Jun 29, 2014 at 4:18 PM, David Rowley dgrowle...@gmail.com wrote: I think I'm finally ready for a review again, so I'll update the commitfest app. I have reviewed this on code level. 1. Patch gets applied cleanly. 2. make/make install/make check all are fine No issues found till

Re: [HACKERS] add line number as prompt option to psql

2014-06-20 Thread Jeevan Chalke
Hi Sawada Masahiko, I liked this feature. So I have reviewed it. Changes are straight forward and looks perfect. No issues found with make/make install/initdb/regression. However I would suggest removing un-necessary braces at if, as we have only one statement into it. if (++cur_line =

Re: [HACKERS] pg_dump reporing version of server pg_dump as comments in the output

2014-06-17 Thread Jeevan Chalke
On Tue, Mar 4, 2014 at 11:28 AM, Wang, Jing ji...@fast.au.fujitsu.com wrote: I don't buy your argument. Why isn't verbose option sufficient? Did you read the old thread about this [1]? [1] http://www.postgresql.org/message-id/3677.1253912...@sss.pgh.pa.us AFAICS a lot of people compare

  1   2   >