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’

[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 as

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

2016-08-30 Thread Jeevan Chalke
On Tue, Aug 30, 2016 at 6:51 PM, Pavel Stehule wrote: > Hi > > 2016-08-30 15:02 GMT+02:00 Jeevan Chalke : > >> Hi all, >> >> Attached is the patch which adds support to push down aggregation and >> grouping >> to the foreign server for postgres_fdw. P

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 fi

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 when we can preliminary

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 f

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] 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-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] 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 (alvhe...@2ndquadrant.com) wrote: > > >>> Can't you keep those words as Sconst

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 and

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 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: > > > > 1. Patch a

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 both

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-09 Thread Jeevan Chalke
end paths and then adds > finalization > path if necessary. The code to add finalization 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

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-09 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 > wrote: > > On 10 October 2017 at 01:10, Jeevan Chalke > > wrote: > >> Attached new patch set having HEAD at 84ad4

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-12 Thread Jeevan Chalke
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

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-15 Thread Jeevan Chalke
; 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 http://www.2nd

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-17 Thread Jeevan Chalke
On Tue, Oct 17, 2017 at 7:13 PM, Dilip Kumar wrote: > On Fri, Oct 13, 2017 at 12:06 PM, Jeevan Chalke > wrote: > > > While playing around with the patch I have noticed one regression with > the partial partition-wise aggregate. > > I am consistently able to reproduce

Re: [HACKERS] Partition-wise aggregation/grouping

2017-10-27 Thread Jeevan Chalke
ation 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 wrote: > On Tue, Oct 17, 2017 at 10:44 PM, Jeevan Chalke > wrote: > > > > > I didn't get what you mean by regression here. Can yo

Re: [HACKERS] Partition-wise aggregation/grouping

2017-11-01 Thread Jeevan Chalke
On Sat, Oct 28, 2017 at 3:07 PM, Robert Haas wrote: > On Fri, Oct 27, 2017 at 1:01 PM, Jeevan Chalke > wrote: > > 1. Added separate patch for costing Append node as discussed up-front in > the > > patch-set. > > 2. Since we now cost Append node, we don't need &g

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] 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] 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 d

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 f

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 perfectly sane; in fac

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 regression tests. That's a di

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

2016-11-08 Thread Jeevan Chalke
I think 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 p

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] 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] 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 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 pg_dump diffs. If we ap

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 >= INT

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 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 now. However some c

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

2014-07-07 Thread Jeevan Chalke
On Sun, Jul 6, 2014 at 10:48 PM, Sawada Masahiko wrote: > On Fri, Jun 20, 2014 at 7:17 PM, Jeevan Chalke > wrote: > > Hi Sawada Masahiko, > > > > I liked this feature. So I have reviewed it. > > > > Changes are straight forward and looks perfect. > > No i

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

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] 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

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 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 number has > re

Re: [HACKERS] Partition-wise aggregation/grouping

2017-03-23 Thread Jeevan Chalke
On Tue, Mar 21, 2017 at 1:47 PM, Antonin Houska wrote: > Jeevan Chalke wrote: > > > Declarative partitioning is supported in PostgreSQL 10 and work is > already in > > progress to support partition-wise joins. Here is a proposal for > partition-wise > > agg

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

2017-08-08 Thread Jeevan Chalke
rogress in terminal 1. SELECT pg_terminate_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 Cor

Re: [HACKERS] Partition-wise aggregation/grouping

2017-04-27 Thread Jeevan Chalke
nd 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] https://commitfest.postgresql.org/14/994/ > >

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

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

2017-09-05 Thread Jeevan Chalke
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@postgresql.org) > To make

Re: [HACKERS] WIP: Aggregation push-down

2017-09-07 Thread Jeevan Chalke
& 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] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

2017-09-08 Thread Jeevan Chalke
argvariable = plpgsql_build_variable((argnames && argnames[i][0] != '\0') ? +argnames[i] : buf, +0, argdtype, false); This requires no new variable and thus no more changes el

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-11 Thread Jeevan Chalke
Hi Pavel, On Sat, Sep 9, 2017 at 11:42 AM, Pavel Stehule wrote: > Hi > > 2017-09-08 9:36 GMT+02:00 Jeevan Chalke : > >> Hi Pavel, >> I like the idea of using parameter name instead of $n symbols. >> >> However, I am slightly worried that, at execution time

Re: [HACKERS] Constraint exclusion for partitioned tables

2017-09-12 Thread Jeevan Chalke
cessarily cascade. > > For partitioning, it may be that we've got enough restrictions in > place on what can happen that we can assume everything can cascade. > Actually, I hope that's true, since the partitioned table has no > storage of its own. > > -- > Robert

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 com> wrote: > >> Here are the new patch-set re-based on HEAD (f0a0c17) and >> latest partition-wise join (v29) pa

Re: [HACKERS] Constraint exclusion for partitioned tables

2017-09-12 Thread Jeevan Chalke
On Tue, Sep 12, 2017 at 8:12 PM, Robert Haas wrote: > On Tue, Sep 12, 2017 at 7:08 AM, Jeevan Chalke > wrote: > > This patch clearly improves the planning time with given conditions. > > > > To verify that, I have created a table like: > > create table foo(a int,

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] Partition-wise join for join between (declaratively) partitioned tables

2017-09-20 Thread Jeevan Chalke
able 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 w

Re: [HACKERS] Partition-wise aggregation/grouping

2017-09-21 Thread Jeevan Chalke
se, but testcase is working as expected. However running those 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 aggregation/grouping

2017-09-27 Thread Jeevan Chalke
be worthwhile 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 reviewin

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 wrote: > > "Kyotaro" == Kyotaro HORIGUCHI > 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 is a List is not enough > to tell whether t

[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 thi

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 D

Re: [HACKERS] [PATCH] Filter error log statements by sqlstate

2014-01-13 Thread Jeevan Chalke
On Mon, Jan 13, 2014 at 4:30 PM, Oskari Saarenmaa wrote: > Hi, > > > On 13/01/14 10:26, Jeevan Chalke wrote: > >> 1. Documentation is missing and thus becomes difficult to understand what >> exactly you are trying to do. Or in other words, user will be uncer

Re: [HACKERS] [BUGS] surprising to_timestamp behavior

2014-01-13 Thread Jeevan Chalke
On Thu, Oct 31, 2013 at 10:50 AM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > > > > On Tue, Oct 29, 2013 at 11:05 PM, Robert Haas wrote: > >> On Tue, Oct 29, 2013 at 12:03 PM, Tom Lane wrote: >> > Robert Haas writes: >> >> It turns ou

Re: [HACKERS] patch: option --if-exists for pg_dump

2014-01-16 Thread Jeevan Chalke
Hi Pavel, I have reviewed the patch and here are my concerns and notes: POSITIVES: --- 1. Patch applies with some white-space errors. 2. make / make install / make check is smooth. No issues as such. 3. Feature looks good as well. 4. NO concern on overall design. 5. Good work. NEGATIVES: --- H

Re: [HACKERS] [PATCH] Filter error log statements by sqlstate

2014-01-16 Thread Jeevan Chalke
Hi Oskari, Patch looks good to me now. I have found no issues too. It is good to go in now. However, few small suggestions: 1. Whenever we know that a variable is containing only 32 bits, better define it as uint32 and not just int (m_sqlstate in get_sqlstate_error_level() function). int size ma

Re: [HACKERS] [BUGS] surprising to_timestamp behavior

2014-01-19 Thread Jeevan Chalke
> > I went to review this, and found that there's not actually a patch > attached ... > > regards, tom lane > Attached. Sorry for that. -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company diff --gi

Re: [HACKERS] patch: option --if-exists for pg_dump

2014-01-21 Thread Jeevan Chalke
/ Looks like you need to re-phrase these comments line. Something like: /* * Object description is based on dropStmt statement which may have * IF EXISTS clause. Thus we need to update an offset such that it * won't be included in the object description.

Re: [HACKERS] patch: option --if-exists for pg_dump

2014-01-29 Thread Jeevan Chalke
Hi Pavel, Now the patch looks good to me. However when I try to restore your own sql file's dump, I get following errors: pg_restore: [archiver (db)] could not execute query: ERROR: relation "public.emp" does not exist Command was: DROP TRIGGER IF EXISTS emp_insert_trigger ON public.emp; pg

Re: [HACKERS] patch: option --if-exists for pg_dump

2014-01-30 Thread Jeevan Chalke
Hi Pavel, it should be fixed by >> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b152c6cd0de1827ba58756e24e18110cf902182acommit >> > Ok. Good. Sorry I didn't update my sources. Done now. Thanks > >> >>> >>> Also, I didn't quite understand these lines of comments: >>> >>>

Re: [HACKERS] [PATCH] Filter error log statements by sqlstate

2014-01-30 Thread Jeevan Chalke
Hi Oskari, Are you planning to work on what Tom has suggested ? It make sense to me as well. What are your views on that ? Thanks -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company

Re: [HACKERS] patch: option --if-exists for pg_dump

2014-01-30 Thread Jeevan Chalke
Hi Pavel, Now patch looks good to me and I think it is in good shape to pass it on to the committer as well. However, I have - Tweaked few comments - Removed white-space errors - Fixed typos - Fixed indentation Attached patch with my changes. However entire design and code logic is untouched. P

Re: [HACKERS] patch: option --if-exists for pg_dump

2014-01-30 Thread Jeevan Chalke
OK. Assigned it to committer. Thanks for the hard work. On Thu, Jan 30, 2014 at 6:16 PM, Pavel Stehule wrote: > Hello > > All is ok > > Thank you > > Pavel > -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company

Re: [HACKERS] patch: option --if-exists for pg_dump

2014-02-02 Thread Jeevan Chalke
, but still you got unstable build. NOT sure how. Seems like you are applying wrong patch. Will you please let us know what's going wrong ? Thanks On Thu, Jan 30, 2014 at 6:56 PM, Pavel Stehule wrote: > > > > 2014-01-30 Jeevan Chalke : > > OK. >> >> Assigned it

Re: [HACKERS] patch: option --if-exists for pg_dump

2014-02-17 Thread Jeevan Chalke
On Mon, Feb 17, 2014 at 7:43 PM, Alvaro Herrera wrote: > Jeevan Chalke escribió: > > > If yes, then in my latest attached patch, these lines are NOT AT ALL > there. > > I have informed on my comment that I have fixed these in my version of > > patch, > > but still

Re: [HACKERS] PL/pgSQL, RAISE and error context

2013-09-17 Thread Jeevan Chalke
Hi Marko, I have reviewed this patch. 1. Patch applies well. 2. make and make install is fine 3. make check is fine too. But as Peter pointed out plperl regression tests are failing. I just did grep on .sql files and found following files which has RAISE statement into it. These files too need

Re: [HACKERS] proposal: simple date constructor from numeric values

2013-09-17 Thread Jeevan Chalke
Hi Pavel, I have reviewed your patch. Patch looks excellent and code changes match with similar constructs elsewhere. That's great. However, it was not applying with git apply command but able to apply it with patch -p1 with some offsets. make and make install was smooth too. Regression suite di

Re: [HACKERS] proposal: simple date constructor from numeric values

2013-09-18 Thread Jeevan Chalke
On Wed, Sep 18, 2013 at 9:54 PM, Pavel Stehule wrote: > Hello > > thank you, > > I have no comments > Thanks. Assigned it to committer. > > Regards > > Pavel > > -- Jeevan B Chalke

Re: [HACKERS] [PATCH] Revive line type

2013-09-25 Thread Jeevan Chalke
Hi, I had a look over this patch and here are my review points: 1. Patch applies cleanly. 2. make, make install and make check is good. 3. I did lot of random testing and didn't find any issue. 4. Test coverage is very well. It has all scenarios and all operators are tested with line. That's real

Re: [HACKERS] [PATCH] Revive line type

2013-10-03 Thread Jeevan Chalke
On Wed, Oct 2, 2013 at 6:12 AM, Peter Eisentraut wrote: > On Wed, 2013-09-25 at 14:26 +0530, Jeevan Chalke wrote: > > So no issues from my side. > > > > However, do we still need this in close_pl() ? > > > > #ifdef NOT_USED > > if

Re: [HACKERS] [PATCH] Revive line type

2013-10-08 Thread Jeevan Chalke
On Wed, Oct 9, 2013 at 10:44 AM, Peter Eisentraut wrote: > On Thu, 2013-10-03 at 17:50 +0530, Jeevan Chalke wrote: > > Will you please attach new patch with above block removed ? Then I > > will quickly check that new patch and mark as "Ready For Committer". > >

[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 the

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 determine the output col

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

2015-09-23 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 default collation. > >

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. With your explanation h

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 wrote: > Jeevan Chalke writes: > > On Wed, Sep 23, 2015 at 10:15 PM, Tom Lane wrote: > >> After a bit more thinking and experimentation, I propose the attached > >> patch. > > > I had a look over the patch and revi

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 saying we are getting a crash with

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 >> trivially easy right now

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 outer > > tuple. > > > > I have no objection to this and

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 > wrote: > >> On Thu, Oct 15, 2015 at 3:04 AM, Kyotaro HORIGUCHI >> wrote: >> > I confirmed that an

Re: [HACKERS] Department of Redundancy Department: makeNode(FuncCall) division

2013-06-24 Thread Jeevan Chalke
Hi David, I hope this is the latest patch to review, right ? I am going to review it. I have gone through the discussion on this thread and I agree with Stephen Frost that it don't add much improvements as such but definitely it is going to be easy for contributors in this area as they don't nee

Re: [HACKERS] [Review] Add SPI_gettypmod() to return a field's typemod from a TupleDesc

2013-06-25 Thread Jeevan Chalke
Hi Mark, Is this the latest patch you are targeting for 9.4 CF1 ? I am going to review it. >From the comment, here is one issue you need to resolve first: *** exec_eval_datum(PLpgSQL_execstate *estat *** 4386,4396 errmsg("record \"%s\" has no fiel

Re: [HACKERS] Department of Redundancy Department: makeNode(FuncCall) division

2013-06-25 Thread Jeevan Chalke
On Tue, Jun 25, 2013 at 11:11 AM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > Hi David, > > I hope this is the latest patch to review, right ? > > I am going to review it. > > I have gone through the discussion on this thread and I agree with Stephen >

Re: [HACKERS] [Review] Add SPI_gettypmod() to return a field's typemod from a TupleDesc

2013-06-26 Thread Jeevan Chalke
On Wed, Jun 26, 2013 at 7:49 AM, Mark Wong wrote: > On Tue, Jun 25, 2013 at 1:38 AM, Jeevan Chalke > wrote: > > Hi Mark, > > > > Is this the latest patch you are targeting for 9.4 CF1 ? > > > > I am going to review it. > > > > From the comm

Re: [HACKERS] checking variadic "any" argument in parser - should be array

2013-06-26 Thread Jeevan Chalke
Hi Pavel On Sat, Jan 26, 2013 at 9:22 AM, Pavel Stehule wrote: > Hello Tom > > you did comment > > ! <><--><--> * Non-null argument had better be an array. > The parser doesn't > ! <><--><--> * enforce this for VARIADIC ANY functions > (maybe it should?), so > ! <><--

Re: [HACKERS] checking variadic "any" argument in parser - should be array

2013-06-27 Thread Jeevan Chalke
rched for VARIADIC and all related documentation says it needs an array, so nothing harmful as such, so you can ignore this review comment but I thought it worth mentioning it. Thanks On Thu, Jun 27, 2013 at 12:35 AM, Pavel Stehule wrote: > Hello > > remastered version > > Regards

Re: [HACKERS] checking variadic "any" argument in parser - should be array

2013-06-28 Thread Jeevan Chalke
Hi Pavel, On Fri, Jun 28, 2013 at 2:53 AM, Pavel Stehule wrote: > Hello > > 2013/6/27 Jeevan Chalke : > > Hi Pavel, > > > > I had a look over your new patch and it looks good to me. > > > > My review comments on patch: > > > > 1. It clean

Re: [HACKERS] Department of Redundancy Department: makeNode(FuncCall) division

2013-07-01 Thread Jeevan Chalke
On Mon, Jul 1, 2013 at 6:16 AM, David Fetter wrote: > On Fri, Jun 28, 2013 at 01:28:35PM -0400, Peter Eisentraut wrote: > > On 6/28/13 11:30 AM, Robert Haas wrote: > > > On Fri, Jun 28, 2013 at 10:31 AM, Tom Lane wrote: > > >> David Fetter writes: > > >>> Please find attached the latest patch.

Re: [HACKERS] checking variadic "any" argument in parser - should be array

2013-07-02 Thread Jeevan Chalke
On Mon, Jul 1, 2013 at 8:36 PM, Pavel Stehule wrote: > 2013/6/29 Pavel Stehule : > > Hello > > > > updated patch - precious Assert, more comments > > > > Regards > > > > Pavel > > > > stripped > Thanks. Patch looks good to me now. Revalidated and didn't see any issue so marking "Ready For Commit

[HACKERS] Regex pattern with shorter back reference does NOT work as expected

2013-07-10 Thread Jeevan Chalke
Hi Tom, Following example does not work as expected: -- Should return TRUE but returning FALSE SELECT 'Programmer' ~ '(\w).*?\1' as t; -- Should return P, a and er i.e. 3 rows but returning just one row with -- value Programmer SELECT REGEXP_SPLIT_TO_TABLE('Programmer','(\w).*?\1'); Initially I

Re: [HACKERS] Regex pattern with shorter back reference does NOT work as expected

2013-07-15 Thread Jeevan Chalke
Hi Tom, On Sat, Jul 13, 2013 at 10:43 PM, Tom Lane wrote: > I wrote: > > Jeevan Chalke writes: > >> Following example does not work as expected: > >> > >> -- Should return TRUE but returning FALSE > >> SELECT 'Programmer' ~ '(\w).*?\

[HACKERS] REGEXP_MATCHES() strange behavior with '^' and '$' pattern

2013-07-31 Thread Jeevan Chalke
Hi, While playing with regular expression I found some strange behavior of regexp_matches() function. Consider following sql query and its output: postgres=# select regexp_matches('1' || chr(10) || '2' || chr(10) || '3' || chr(10) || '4', '^', 'mg'); regexp_matches {""} {""}

Re: [HACKERS] REGEXP_MATCHES() strange behavior with '^' and '$' pattern

2013-07-31 Thread Jeevan Chalke
Oops forgot patch. Attached now. On Wed, Jul 31, 2013 at 6:03 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > Hi, > > While playing with regular expression I found some strange behavior of > regexp_matches() function. > > Consider following sql query and

Re: [HACKERS] REGEXP_MATCHES() strange behavior with '^' and '$' pattern

2013-07-31 Thread Jeevan Chalke
On Wed, Jul 31, 2013 at 7:47 PM, Tom Lane wrote: > Jeevan Chalke writes: > > Oops forgot patch. > > Attached now. > > Hmm ... I think the logic change is good, but two demerits for not fixing > the adjacent comment. > I had a look over comments and somehow I foun

Re: [HACKERS] REGEXP_MATCHES() strange behavior with '^' and '$' pattern

2013-08-01 Thread Jeevan Chalke
On Thu, Aug 1, 2013 at 12:25 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > > > > On Wed, Jul 31, 2013 at 7:47 PM, Tom Lane wrote: > >> Jeevan Chalke writes: >> > Oops forgot patch. >> > Attached now. >> >> Hmm ... I think

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

  1   2   >