Thread: [HACKERS] Passing query string to workers
Hello everybody,
Currently, query string is not passed to the workers and only master has it. In the events, when multiple queries are running on a server and for one query some worker crashes then it becomes quite burdensome to find the query with the crashed worker, since on the worker crash no query is displayed.
To fix this, I propose a patch wherein query string is passed to the workers as well, hence, displayed when worker crashes.
Approach:
A token for query string is created in the shared memory, this token is populated with the query string using the global string -- debug_query_string. Now, for each of the worker when ExecGetParallelQueryDesc is called, we retrieve the query text from shared memory and pass it to CreateQueryDesc.
Next, to ensure that query gets displayed at the time of crash, BackendStatusArray needs to be populated correctly, specifically for our purpose, activity needs to be filled with current query. For this I called pgstat_report_activity in ParallelWorkerMain, with the query string, this populates workers' tuples in system table -- pgstat_activity as well. Previously, pgstat_report_activity was only called for master in exec_simple_query, hence, for workers pgstat_activity remained null.
Results:
Currently, query string is not passed to the workers and only master has it. In the events, when multiple queries are running on a server and for one query some worker crashes then it becomes quite burdensome to find the query with the crashed worker, since on the worker crash no query is displayed.
To fix this, I propose a patch wherein query string is passed to the workers as well, hence, displayed when worker crashes.
Approach:
A token for query string is created in the shared memory, this token is populated with the query string using the global string -- debug_query_string. Now, for each of the worker when ExecGetParallelQueryDesc is called, we retrieve the query text from shared memory and pass it to CreateQueryDesc.
Next, to ensure that query gets displayed at the time of crash, BackendStatusArray needs to be populated correctly, specifically for our purpose, activity needs to be filled with current query. For this I called pgstat_report_activity in ParallelWorkerMain, with the query string, this populates workers' tuples in system table -- pgstat_activity as well. Previously, pgstat_report_activity was only called for master in exec_simple_query, hence, for workers pgstat_activity remained null.
Results:
Here is an output for artificially created worker crash with and without the patch.
Without the patch error report on worker crash:
LOG: worker process: parallel worker for PID 49739 (PID 49741) was terminated by signal 11: Segmentation fault
Error report with the patch:
Inputs of all sorts are encouraged.
--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
LOG: worker process: parallel worker for PID 49739 (PID 49741) was terminated by signal 11: Segmentation fault
Error report with the patch:
LOG: worker process: parallel worker for PID 51757 (PID 51758) was terminated by signal 11: Segmentation fault
2017-01-11 11:10:27.630 IST [51742] DETAIL: Failed process was running: explain analyse select
l_returnflag,
l_linestatus,
sum(l_quantity) as sum_qty,
sum(l_extendedprice) as sum_base_price,
sum(l_extendedprice * (1 - l_discount)) as sum_disc_price,
sum(l_extendedprice * (1 - l_discount) * (1 + l_tax)) as sum_charge,
avg(l_quantity) as avg_qty,
avg(l_extendedprice) as avg_price,
avg(l_discount) as avg_disc,
count(*) as count_order
from
lineitem
where
l_shipdate <= date '1998-12-01' - interval '119' day
group by
l_returnflag,
l_linestatus
order by
l_returnflag,
l_linestatus
LIMIT 1;
Inputs of all sorts are encouraged.
--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
Attachment
Rafia Sabih <rafia.sabih@enterprisedb.com> writes:
> Approach:
> A token for query string is created in the shared memory, this token is
> populated with the query string using the global string --
> debug_query_string. Now, for each of the worker when
> ExecGetParallelQueryDesc is called, we retrieve the query text from shared
> memory and pass it to CreateQueryDesc.
This is simply wrong, because debug_query_string need have nothing to do
with what the actual parallel query is. I'm all for sending over the
correct query, but if you aren't bothering to accurately reproduce the
master's query descriptor, I think you will increase confusion not
reduce it.
As far as reproducing the pg_stat_activity query goes, you could probably
grab that string out of the master backend's pgstat entry and pass it over
at parallel query start. But please don't confuse it with either
debug_query_string or the string referenced by the QueryDesc; I do not
think there's any guarantee that those are the same.
regards, tom lane
On Wed, Jan 11, 2017 at 7:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Rafia Sabih <rafia.sabih@enterprisedb.com> writes: >> Approach: >> A token for query string is created in the shared memory, this token is >> populated with the query string using the global string -- >> debug_query_string. Now, for each of the worker when >> ExecGetParallelQueryDesc is called, we retrieve the query text from shared >> memory and pass it to CreateQueryDesc. > > This is simply wrong, because debug_query_string need have nothing to do > with what the actual parallel query is. I'm all for sending over the > correct query, but if you aren't bothering to accurately reproduce the > master's query descriptor, I think you will increase confusion not > reduce it. > > As far as reproducing the pg_stat_activity query goes, you could probably > grab that string out of the master backend's pgstat entry and pass it over > at parallel query start. But please don't confuse it with either > debug_query_string or the string referenced by the QueryDesc; I do not > think there's any guarantee that those are the same. I think we should pass only the string referenced by the QueryDesc to the worker, and on the worker side report that via debug_query_string and pg_stat_activity and attach it to the QueryDesc itself. Is that also your view? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jan 11, 2017 at 7:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> As far as reproducing the pg_stat_activity query goes, you could probably
>> grab that string out of the master backend's pgstat entry and pass it over
>> at parallel query start. But please don't confuse it with either
>> debug_query_string or the string referenced by the QueryDesc; I do not
>> think there's any guarantee that those are the same.
> I think we should pass only the string referenced by the QueryDesc to
> the worker, and on the worker side report that via debug_query_string
> and pg_stat_activity and attach it to the QueryDesc itself. Is that
> also your view?
That would work, if you had a way to get at the active QueryDesc ...
but we don't pass that down to executor nodes.
regards, tom lane
On Wed, Jan 11, 2017 at 6:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Wed, Jan 11, 2017 at 7:37 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> As far as reproducing the pg_stat_activity query goes, you could probably >>> grab that string out of the master backend's pgstat entry and pass it over >>> at parallel query start. But please don't confuse it with either >>> debug_query_string or the string referenced by the QueryDesc; I do not >>> think there's any guarantee that those are the same. > >> I think we should pass only the string referenced by the QueryDesc to >> the worker, and on the worker side report that via debug_query_string >> and pg_stat_activity and attach it to the QueryDesc itself. Is that >> also your view? > > That would work, if you had a way to get at the active QueryDesc ... > but we don't pass that down to executor nodes. Hmm, that is a bit of a problem. Do you have a suggestion? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jan 11, 2017 at 6:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> That would work, if you had a way to get at the active QueryDesc ...
>> but we don't pass that down to executor nodes.
> Hmm, that is a bit of a problem. Do you have a suggestion?
Copy that string pointer into the EState, perhaps?
regards, tom lane
On Wed, Jan 11, 2017 at 11:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Wed, Jan 11, 2017 at 6:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> That would work, if you had a way to get at the active QueryDesc ... >>> but we don't pass that down to executor nodes. > >> Hmm, that is a bit of a problem. Do you have a suggestion? > > Copy that string pointer into the EState, perhaps? OK, that sounds good. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Jan 12, 2017 at 6:24 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Wed, Jan 11, 2017 at 11:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Robert Haas <robertmhaas@gmail.com> writes: >>> On Wed, Jan 11, 2017 at 6:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>>> That would work, if you had a way to get at the active QueryDesc ... >>>> but we don't pass that down to executor nodes. >> >>> Hmm, that is a bit of a problem. Do you have a suggestion? >> >> Copy that string pointer into the EState, perhaps? > > OK, that sounds good. Thanks a lot Tom and Robert for the suggestions. Please find the attached file for the revised version of the patch, wherein I added an extra pointer for queryString in EState and populated it with queryDesc->sourceText in standard_ExecutorStart. Later, in ExecInitParallelPlan a token for queryString is created in shared memory which is used in ExecParallelGetQueryDesc and ParallelWorkerMain as before (in version 1 of patch). Please let me know your feedback over the same. -- Regards, Rafia Sabih EnterpriseDB: http://www.enterprisedb.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On Fri, Jan 13, 2017 at 4:25 PM, Rafia Sabih <rafia.sabih@enterprisedb.com> wrote: > > Please let me know your feedback over the same. > I've looked into the patch. I've some comments regarding that. +#define PARALLEL_KEY_QUERY_TEXT UINT64CONST(0xE000000000000010) It should be UINT64CONST(0xE00000000000000A) + query_len = strlen(query_data) + 1; + shm_toc_estimate_chunk(&pcxt->estimator, query_len); + shm_toc_estimate_keys(&pcxt->estimator, 1); + Adding a comment before this will be helpful. You should maintain the same order for estimating and storing the query string. For example, as you've estimated the space for query string after estimation of dsa space, you should store the same after creating dsa. Besides, I think the appropriate place for this would be before planned statement. The regression test for select_parallel is failing after applying the patch. You can reproduce the scenario by the following sql statements: CREATE TABLE t1(a int); INSERT INTO t1 VALUES (generate_series(1,100000)); SET parallel_setup_cost=0; SET parallel_tuple_cost=0; SET min_parallel_relation_size=0; SET max_parallel_workers_per_gather=4; SELECT count(*) FROM t1; It is showing the following warning. WARNING: problem in alloc set ExecutorState: detected write past chunk end in block 0x14f5310, chunk 0x14f6c50 -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
On Mon, Jan 23, 2017 at 2:46 PM, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote: > I've looked into the patch. I've some comments regarding that. > > +#define PARALLEL_KEY_QUERY_TEXT UINT64CONST(0xE000000000000010) > It should be UINT64CONST(0xE00000000000000A) > > + query_len = strlen(query_data) + 1; > + shm_toc_estimate_chunk(&pcxt->estimator, query_len); > + shm_toc_estimate_keys(&pcxt->estimator, 1); > + > Adding a comment before this will be helpful. You should maintain the > same order for estimating and storing the query string. For example, > as you've estimated the space for query string after estimation of dsa > space, you should store the same after creating dsa. Besides, I think > the appropriate place for this would be before planned statement. Fixed. > The regression test for select_parallel is failing after applying the > patch. You can reproduce the scenario by the following sql statements: > > CREATE TABLE t1(a int); > INSERT INTO t1 VALUES (generate_series(1,100000)); > SET parallel_setup_cost=0; > SET parallel_tuple_cost=0; > SET min_parallel_relation_size=0; > SET max_parallel_workers_per_gather=4; > SELECT count(*) FROM t1; > > It is showing the following warning. > WARNING: problem in alloc set ExecutorState: detected write past > chunk end in block 0x14f5310, chunk 0x14f6c50 Fixed. Thanks a lot Kuntal for the review, please find attached patch for the revised version. -- Regards, Rafia Sabih EnterpriseDB: http://www.enterprisedb.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On Tue, Feb 7, 2017 at 10:19 AM, Rafia Sabih <rafia.sabih@enterprisedb.com> wrote: > Thanks a lot Kuntal for the review, please find attached patch for the > revised version. Few comments on the patch: There are some spacing issues in the code. For example, + estate->es_queryString = (char *)palloc0(strlen(queryDesc->sourceText) + 1); + /*Estimate space for query text. */ pgindent might be helpful to track all such changes. +#define PARALLEL_KEY_QUERY_TEXT UINT64CONST(0xE000000000000010) I'm uncomfortable with declaring the same macro in two files(parallel.c, execParallel.c). My suggestion would be to move pgstat_report_activity in ParallelQueryMain instead of ParallelWorkerMain. Then, you can remove the macro definition from parallel.c. Thoughts? And, the value of the macro seems pretty random to me. IMO, it should be UINT64CONST(0xE000000000000007). -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
On Fri, Feb 10, 2017 at 2:54 PM, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote: > On Tue, Feb 7, 2017 at 10:19 AM, Rafia Sabih > <rafia.sabih@enterprisedb.com> wrote: >> Thanks a lot Kuntal for the review, please find attached patch for the >> revised version. > Few comments on the patch: > > There are some spacing issues in the code. For example, > + estate->es_queryString = (char > *)palloc0(strlen(queryDesc->sourceText) + 1); > + /*Estimate space for query text. */ > pgindent might be helpful to track all such changes. > > +#define PARALLEL_KEY_QUERY_TEXT UINT64CONST(0xE000000000000010) > I'm uncomfortable with declaring the same macro in two > files(parallel.c, execParallel.c). My suggestion would be to move > pgstat_report_activity in ParallelQueryMain instead of > ParallelWorkerMain. Then, you can remove the macro definition from > parallel.c. Thoughts? > Yes, I also don't see any need of defining it in parallel.c. I think she has kept to report it in pg_stat_activity, but I feel that code can also be moved to execParallel.c. Another question is don't we need to set debug_query_string in worker? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
>
> There are some spacing issues in the code. For example,
> + estate->es_queryString = (char
> *)palloc0(strlen(queryDesc->sourceText) + 1);
> + /*Estimate space for query text. */
> pgindent might be helpful to track all such changes.
>
Fixed.
> +#define PARALLEL_KEY_QUERY_TEXT UINT64CONST(0xE000000000000010)
> I'm uncomfortable with declaring the same macro in two
> files(parallel.c, execParallel.c). My suggestion would be to move
> pgstat_report_activity in ParallelQueryMain instead of
> ParallelWorkerMain. Then, you can remove the macro definition from
> parallel.c. Thoughts?
>
Yes, I also don't see any need of defining it in parallel.c. I think
she has kept to report it in pg_stat_activity, but I feel that code
can also be moved to execParallel.c.
Agree and fixed.
Another question is don't we need to set debug_query_string in worker?
In the updated version I am setting it in ParallelQueryMain.
Please find the attached file for the revised version.
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
Attachment
On Sat, Feb 11, 2017 at 8:38 AM, Rafia Sabih <rafia.sabih@enterprisedb.com> wrote: > >> >> Another question is don't we need to set debug_query_string in worker? > > In the updated version I am setting it in ParallelQueryMain. > Ahh, I missed that. debug_query_string is used to show the log statements. Hence, it should be set. > Please find the attached file for the revised version. > + queryString = shm_toc_lookup(toc, PARALLEL_KEY_QUERY_TEXT); + debug_query_string = shm_toc_lookup(toc, PARALLEL_KEY_QUERY_TEXT); + pgstat_report_activity(STATE_RUNNING, shm_toc_lookup(toc, PARALLEL_KEY_QUERY_TEXT)); Just one lookup is sufficient. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
On Thu, Feb 16, 2017 at 5:06 PM, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote:
-- >>
>> Another question is don't we need to set debug_query_string in worker?
>
> In the updated version I am setting it in ParallelQueryMain.
>
Ahh, I missed that. debug_query_string is used to show the log
statements. Hence, it should be set.
+ queryString = shm_toc_lookup(toc, PARALLEL_KEY_QUERY_TEXT);
+ debug_query_string = shm_toc_lookup(toc, PARALLEL_KEY_QUERY_TEXT);
+ pgstat_report_activity(STATE_RUNNING, shm_toc_lookup(toc,
PARALLEL_KEY_QUERY_TEXT));
Just one lookup is sufficient.
Fixed.
Other that that I updated some comments and other cleanup things. Please find the attached patch for the revised version.
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
Attachment
On Thu, Feb 16, 2017 at 5:47 PM, Rafia Sabih <rafia.sabih@enterprisedb.com> wrote: > Other that that I updated some comments and other cleanup things. Please > find the attached patch for the revised version. Looks good. It has passed the regression tests (with and without regress mode). Query is getting displayed for parallel workers in pg_stat_activity, log statements and failed-query statements. Moved status to Ready for committer. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
On Thu, Feb 16, 2017 at 6:41 PM, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote: > On Thu, Feb 16, 2017 at 5:47 PM, Rafia Sabih > <rafia.sabih@enterprisedb.com> wrote: >> Other that that I updated some comments and other cleanup things. Please >> find the attached patch for the revised version. > Looks good. > > It has passed the regression tests (with and without regress mode). > Query is getting displayed for parallel workers in pg_stat_activity, > log statements and failed-query statements. Moved status to Ready for > committer. The first hunk of this patch says: + estate->es_queryString = (char *) palloc0(strlen(queryDesc->sourceText) + 1); + estate->es_queryString = queryDesc->sourceText; This is all kinds of wrong. 1. If you assign a value to a variable, and then immediately assign another value to a variable, the first assignment might as well not have happened. In other words, this is allocating a string and then immediately leaking the allocated memory. 2. If the intent was to copy the string in into estate->es_queryString, ignoring for the moment that you'd need to use strcpy() rather than an assignment to make that happen, the use of palloc0 would be unnecessary. Regular old palloc would be fine, because you don't need to zero the memory if you're about to overwrite it with some new contents. Zeroing isn't free, so it's best not to do it unnecessarily. 3. Instead of using palloc and then copying the string, you could just use pstrdup(), which does the whole thing for you. 4. I don't actually think you need to copy the query string at all. Tom's email upthread referred to copying the POINTER to the string, not the string itself, and src/backend/executor/README seems to suggest that FreeQueryDesc can't be called until after ExecutorEnd(). So I think you could replace these two lines of code with estate->es_queryString = queryDesc->sourceText and call it good. That's essentially what this is doing anyway, as the code is written. +/* For passing query text to the workers */ Unnecessary, because it's clear from the name PARALLEL_KEY_QUERY_TEXT. #define PARALLEL_TUPLE_QUEUE_SIZE 65536 -/* Don't remove this blank line. + query_data = (char *) palloc0(strlen(estate->es_queryString) + 1); + strcpy(query_data, estate->es_queryString); It's unnecessary to copy the query string here; you're going to use it later on in the very same function. That code can just refer to estate->es_queryString directly. + const char *es_queryString; /* Query string for passing to workers */ Maybe this should be called es_sourceText, since it's being copied from querydesc->sourceText? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Sun, Feb 19, 2017 at 10:11 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Feb 16, 2017 at 6:41 PM, Kuntal Ghosh
<kuntalghosh.2007@gmail.com> wrote:
> On Thu, Feb 16, 2017 at 5:47 PM, Rafia Sabih
> <rafia.sabih@enterprisedb.com> wrote:
>> Other that that I updated some comments and other cleanup things. Please
>> find the attached patch for the revised version.
> Looks good.
>
> It has passed the regression tests (with and without regress mode).
> Query is getting displayed for parallel workers in pg_stat_activity,
> log statements and failed-query statements. Moved status to Ready for
> committer.
The first hunk of this patch says:
+ estate->es_queryString = (char *)
palloc0(strlen(queryDesc->sourceText) + 1);
+ estate->es_queryString = queryDesc->sourceText;
This is all kinds of wrong.
1. If you assign a value to a variable, and then immediately assign
another value to a variable, the first assignment might as well not
have happened. In other words, this is allocating a string and then
immediately leaking the allocated memory.
2. If the intent was to copy the string in into
estate->es_queryString, ignoring for the moment that you'd need to use
strcpy() rather than an assignment to make that happen, the use of
palloc0 would be unnecessary. Regular old palloc would be fine,
because you don't need to zero the memory if you're about to overwrite
it with some new contents. Zeroing isn't free, so it's best not to do
it unnecessarily.
3. Instead of using palloc and then copying the string, you could just
use pstrdup(), which does the whole thing for you.
4. I don't actually think you need to copy the query string at all.
Tom's email upthread referred to copying the POINTER to the string,
not the string itself, and src/backend/executor/README seems to
suggest that FreeQueryDesc can't be called until after ExecutorEnd().
So I think you could replace these two lines of code with
estate->es_queryString = queryDesc->sourceText and call it good.
That's essentially what this is doing anyway, as the code is written.
Fixed.
+/* For passing query text to the workers */
Unnecessary, because it's clear from the name PARALLEL_KEY_QUERY_TEXT.
True, done.
#define PARALLEL_TUPLE_QUEUE_SIZE 65536
-
/*
Don't remove this blank line.
Done.
+ query_data = (char *) palloc0(strlen(estate->es_queryString) + 1);
+ strcpy(query_data, estate->es_queryString);
It's unnecessary to copy the query string here; you're going to use it
later on in the very same function. That code can just refer to
estate->es_queryString directly.
Done.
+ const char *es_queryString; /* Query string for passing to workers */
Maybe this should be called es_sourceText, since it's being copied
from querydesc->sourceText?
Done.
Please find the attached patch for the revised version.
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
Attachment
On Mon, Feb 20, 2017 at 10:11 AM, Rafia Sabih <rafia.sabih@enterprisedb.com> wrote: > On Sun, Feb 19, 2017 at 10:11 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> + query_data = (char *) palloc0(strlen(estate->es_queryString) + 1); >> + strcpy(query_data, estate->es_queryString); >> >> It's unnecessary to copy the query string here; you're going to use it >> later on in the very same function. That code can just refer to >> estate->es_queryString directly. > > > Done. + char *query_data; + query_data = estate->es_sourceText; estate->es_sourceText is a const char* variable. Assigning this const pointer to a non-const pointer violates the rules constant-correctness. So, either you should change query_data as const char*, or as Robert suggested, you can directly use estate->es_sourceText. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
On Mon, Feb 20, 2017 at 8:35 PM, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote:
-- + char *query_data;
+ query_data = estate->es_sourceText;
estate->es_sourceText is a const char* variable. Assigning this const
pointer to a non-const pointer violates the rules
constant-correctness. So, either you should change query_data as const
char*, or as Robert suggested, you can directly use
estate->es_sourceText.
Done.
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
Attachment
On Tue, Feb 21, 2017 at 9:18 AM, Rafia Sabih <rafia.sabih@enterprisedb.com> wrote: > Done. Looks fine to me. Committed. I did move es_queryText to what I think is a more appropriate location in the structure definition. Thanks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Feb 22, 2017 at 12:25 PM, Robert Haas <robertmhaas@gmail.com> wrote: > Looks fine to me. Committed. I did move es_queryText to what I think > is a more appropriate location in the structure definition. > > Thanks. > Many thanks to Robert for committing and to Kuntal and Amit for reviewing. -- Regards, Rafia Sabih EnterpriseDB: http://www.enterprisedb.com/