Re: [HACKERS] Refactor handling of database attributes between pg_dump and pg_dumpall

[Haribabu Kommi]

On Sun, Oct 22, 2017 at 3:08 AM, Robert Haas  wrote:

> On Sat, Oct 21, 2017 at 1:30 AM, Haribabu Kommi
>  wrote:
> > Before refactoring, pg_dumpall doesn't print "create database" commands
> > for both tempalte1 and postgres database, but on the other hand pg_dump
> > dump the create database commands with --create option.
> >
> > To keep the behavior of all the database attributes in the dump of both
> > pg_dump and pg_dumpall, the code is unified and moved into pg_dump.
> > But to retain the pg_dumpall behavior of not dumping the "create
> database"
> > commands, a new option is added to pg_dump to skip dumping the
> > create database commands.
> >
> > The new option name is now "--skip-create-default-db", this can be used
> > normal user also when try to dump the postgres database to not let create
> > the database commands in the dump.
>
> I don't get this at all.  If you don't want to create the database,
> just don't pass the -C argument.  It doesn't make sense to have a -C
> argument which makes it create the database and then a
> --skip-create-default-db argument which makes it sometimes not create
> the database after all.


Apologies for not providing much details.

pg_dumpall is used to produce the following statements for database,

"Create database" (other than default database) or
"Alter database set tablespace" for default database (if required)

ACL queries related to database
Alter database config
Alter database role config

whereas, pg_dump used to produce only "create database statement".

With the refactoring, all the pg_dumpall database statements are moved
into pg_dump. -C/--create option of pg_dump produces all the statements
of pg_dumpall. The --skip-default-create-db option is to make sure that
it doesn't produce "Create database" statement and instead may produce
"Alter database set tablespace" for default databases of (postgres and
template1).

-C/--create option is to control the entire database statements.
--skip-create-default-db is option to control the "create" or "Alter"
database statement
for default database.

During restore the dump, the -C/--create restores all the Database
statements.

comments? or any better approach?

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] legitimacy of using PG_TRY , PG_CATCH , PG_END_TRY in C function

[Tom Lane]

John Lumby  writes:
> I have a C function (a trigger function) which uses the PG_TRY() 
> construct to handle certain ERROR conditions.
> One example is where invoked as INSTEAD OF INSERT into a view.  It 
> PG_TRYs INSERT into the real base table,
> but this table may not yet exist  (it is a partitioned child of an 
> inheritance parent).
> If the error is  ERRCODE_UNDEFINED_TABLE,  then the CATCH issues 
> FlushErrorState() and returns to caller who CREATes the table and 
> re-issues the insert.
> All works perfectly (on every release of 9.x).

If it works, it's only because you didn't try very hard to break it.
In general you can't catch and ignore errors without a full-fledged
subtransaction --- BeginInternalSubTransaction, then either
ReleaseCurrentSubTransaction or RollbackAndReleaseCurrentSubTransaction,
not just FlushErrorState.  See e.g. plpgpsql's exec_stmt_block.

There may well be specific scenarios where an error gets thrown without
having done anything that requires transaction cleanup.  But when you
hit a scenario where that's not true, or when a scenario that used to
not require cleanup now does, nobody is going to consider that a PG bug.

regards, tom lane


-- 
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] alter table doc fix

[Amit Langote]

On 2017/10/18 20:37, Alvaro Herrera wrote:
> Amit Langote wrote:
>> Hi.
>>
>> Noticed that a alter table sub-command's name in Description (where it's
>> OWNER) differs from that in synopsis (where it's OWNER TO).  Attached
>> patch to make them match, if the difference is unintentional.
> 
> I agree -- pushed.

Thanks for committing.

> This paragraph
> 
>
> The actions for identity columns (ADD
> GENERATED, SET etc., DROP
> IDENTITY), as well as the actions
> TRIGGER, CLUSTER, 
> OWNER,
> and TABLESPACE never recurse to descendant tables;
> that is, they always act as though ONLY were specified.
> Adding a constraint recurses only for CHECK constraints
> that are not marked NO INHERIT.
>
> 
> is a bit annoying, though I think it'd be worse if we "fix" it to be
> completely strict about the subcommands it refers to.

I didn't notice it in this paragraph before you pointed out, but maybe as
you say, there's not much point in trying to be strict here too.  If we do
fix it though, we might want to do something about TRIGGER, CLUSTER, too,
because there are no sub-commands named just TRIGGER, CLUSTER.

Thanks,
Amit



-- 
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] Block level parallel vacuum WIP

[Amit Langote]

On 2017/10/22 5:25, Thomas Munro wrote:
> On Sun, Oct 22, 2017 at 5:09 AM, Robert Haas  wrote:
>> On Tue, Sep 19, 2017 at 3:31 AM, Masahiko Sawada  
>> wrote:
 Down at the bottom of the build log in the regression diffs file you can 
 see:

 ! ERROR: cache lookup failed for relation 32893

 https://travis-ci.org/postgresql-cfbot/postgresql/builds/277165907
>>>
>>> Thank you for letting me know.
>>>
>>> Hmm, it's an interesting failure. I'll investigate it and post the new 
>>> patch.
>>
>> Did you ever find out what the cause of this problem was?
> 
> I wonder if it might have been the same issue that commit
> 19de0ab23ccba12567c18640f00b49f01471018d fixed a week or so later.

Hmm, 19de0ab23ccba seems to prevent the "cache lookup failed for relation
XXX" error in a different code path though (the code path handling manual
vacuum).  Not sure if the commit could have prevented that error being
emitted by DROP SCHEMA ... CASCADE, which seems to be what produced it in
this case.  Maybe I'm missing something though.

Thanks,
Amit



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] legitimacy of using PG_TRY , PG_CATCH , PG_END_TRY in C function

[John Lumby]

I have a C function (a trigger function) which uses the PG_TRY() 
construct to handle certain ERROR conditions.
One example is where invoked as INSTEAD OF INSERT into a view.  It 
PG_TRYs INSERT into the real base table,
but this table may not yet exist  (it is a partitioned child of an 
inheritance parent).
If the error is  ERRCODE_UNDEFINED_TABLE,  then the CATCH issues 
FlushErrorState() and returns to caller who CREATes the table and 
re-issues the insert.
All works perfectly (on every release of 9.x).

But in a different part of the same trigger function,   there is a 
PG_TRY , PG_CATCH of a CREATE INDEX,
which sometimes hits error "relation already exists" (i.e. index with 
same name indexing a different table).
The CATCH issues FlushErrorState() and returns to caller who ignores the 
error.
All works but not perfectly --  at COMMIT,  resource_owner issues  
relcache reference leak messages about relation scans not closed
and also about  snapshot still active. I guess that the CREATE has 
switched resource_owner and pushed a snapshot,  but I did not
debug in detail.

Those are just examples,  but my question is whether use of this 
construct is legal in a C function
(always?   sometimes  e,g only for DML but not DDL?  never?),
and if not,   then how can one legally catch ERRORs like this in a C 
function?
(Yes I realise I could write selects of pg_class to avoid these two 
particular cases rather than the try-and-fail approach
but I would like to understand the general rules where it may be much 
harder to try avoidance)

Cheers,  John


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC calls very much

[Michael Paquier]

On Mon, Oct 23, 2017 at 6:11 AM, Tom Lane  wrote:
> This comment is neither correct nor intelligible:
>
> /* important for value, key cannot being NULL */
>
> I'd say just drop it.

Yep.

> The checks for "could not determine data type" errors seem
> rather duplicative, too.

Yep.

> 
> The extern declaration seems to have been dropped in a rather
> random place in the .h file.
> 

funcapi.h/c are nicely documented. I think that more is needed.

> Looks good otherwise.

My set of diffs for funcapi.h are actually that:
  * funcapi.h
  *   Definitions for functions which return composite type and/or sets
+ *   or work on VARIADIC inputs.
[...]
+/*--
+ * Support to ease writing of functions dealing with VARIADIC inputs
+ *--
+ *
+ * This function extracts a set of argument values, types and NULL markers
+ * for a given input function. This returns a set of data:
+ * - **values includes the set of Datum values extracted.
+ * - **types the data type OID for each element.
+ * - **nulls tracks if an element is NULL.
+ *
+ * convert_unknown set to true enforces conversion of unknown input type
+ * unknown to text.
+ * variadic_start tracks the argument number of the function call where the
+ * VARIADIC set of arguments begins.
+ *
+ * The return result is the number of elements stored. In the event of a
+ * NULL input, then the caller of this function ought to generate a NULL
+ * object as final result, and in this case a result value of -1 is used
+ * to be able to make the difference between an empty array or object.
+ */
+extern int extract_variadic_args(FunctionCallInfo fcinfo, int variadic_start,
+bool convert_unknown, Datum **values,
Oid **types,
+bool **nulls);

Got this bit as well:
  * funcapi.c
  *   Utility and convenience functions for fmgr functions that return
- *   sets and/or composite types.
+ *   sets and/or composite types, or deal with VARIADIC inputs.
  *
-- 
Michael


-- 
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] How to determine that a TransactionId is really aborted?

[Eric Ridge]

> On Oct 22, 2017, at 3:24 PM, Peter Geoghegan  wrote:
> 
> On Sun, Oct 22, 2017 at 2:19 PM, Eric Ridge  wrote:
>> I'm looking for the status as any concurrent open transaction might see it.  
>> For example, if any concurrent transaction might see it as "in progress", 
>> that's what I'd want returned.  Does that make sense?
> 
> Maybe, but note that that's fundamentally something that can become
> stale immediately. And, just because an MVCC snapshot cannot see a row
> does not mean that it cannot affect its transaction/statement in some
> other way (e.g. unique index enforcement).

Sure, but I don't think I care if it becomes stale immediately.  If I ask "now" 
and PG says "in progress" but it then aborts a few cycles later, I'll just ask 
again sometime in the future.  It's not like a transaction is going to go from 
"aborted" back to "in progress" -- geez, at least I hope not!

I think question I want to answer is "will all active and future transactions 
see this TransationId as aborted at the time they started"?

> Again, you'll probably need to put this low level requirement into
> context if you want sound advice from this list.

I'm just thinking out lout here, but the context is likely something along the 
lines of externally storing all transaction ids, and periodically asking 
Postgres if they're known-to-be-aborted-by-all-transactions -- one at a time.

eric

-- 
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] How to determine that a TransactionId is really aborted?

[Peter Geoghegan]

On Sun, Oct 22, 2017 at 2:19 PM, Eric Ridge  wrote:
> I'm looking for the status as any concurrent open transaction might see it.  
> For example, if any concurrent transaction might see it as "in progress", 
> that's what I'd want returned.  Does that make sense?

Maybe, but note that that's fundamentally something that can become
stale immediately. And, just because an MVCC snapshot cannot see a row
does not mean that it cannot affect its transaction/statement in some
other way (e.g. unique index enforcement).

Again, you'll probably need to put this low level requirement into
context if you want sound advice from this list.

-- 
Peter Geoghegan


-- 
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] How to determine that a TransactionId is really aborted?

[Eric Ridge]

> On Oct 22, 2017, at 2:50 PM, Jaime Casanova  
> wrote:
> 
> so, what you want is txid_status() [1]... while this is new in v10 you
> can use the code as guide or just migrate to v10 ;)

Oh neat, thanks.  



Doesn't that tell you the status relative to the transaction calling 
txid_status()?  

I'm looking for the status as any concurrent open transaction might see it.  
For example, if any concurrent transaction might see it as "in progress", 
that's what I'd want returned.  Does that make sense?  

That's why I was thinking GetOldestXmin() was the right thing to use rather 
than GetActiveSnapshot()->xmin and also why I thought to check 
TransactionIdPrecedes() first.

I am curious about the lock on ClogTruncationLock... could any of the 
TransactionIdDidXXX calls lie without that lock?  I haven't seen such a thing 
used in the 9.3 sources.  Maybe it's necessary for 10 or maybe I just missed it 
in 9.3?

Thanks for your time!

eric

-- 
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] [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC calls very much

[Tom Lane]

Andrew Dunstan  writes:
> Sorry. Here it is.

This comment is neither correct nor intelligible:

/* important for value, key cannot being NULL */

I'd say just drop it.

The checks for "could not determine data type" errors seem
rather duplicative, too.


The extern declaration seems to have been dropped in a rather
random place in the .h file.


Looks good otherwise.

regards, tom lane


-- 
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] How to determine that a TransactionId is really aborted?

[Jaime Casanova]

On 22 October 2017 at 15:00, Eric Ridge  wrote:
>> On Oct 22, 2017, at 1:50 PM, Peter Geoghegan  wrote:
>>
>> On Sun, Oct 22, 2017 at 12:23 PM, Eric Ridge  wrote:
>>> Can anyone confirm or deny that this is correct?  I feel like it is 
>>> correct, but I'm no expert.
>>
>> What are you going to use the code for? I think that that context is
>> likely to matter here.
>
> I'm not exactly sure yet, but I'm thinking about storing transaction ids 
> externally and then regularly poking Postgres to see which ones are 
> aborted-and-no-longer-considered-visible so I can clean up my external list.
>

so, what you want is txid_status() [1]... while this is new in v10 you
can use the code as guide or just migrate to v10 ;)

[1] 
https://www.postgresql.org/docs/10/static/functions-info.html#functions-txid-snapshot

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC calls very much

[Andrew Dunstan]

On 10/22/2017 04:35 PM, Michael Paquier wrote:
> On Mon, Oct 23, 2017 at 1:44 AM, Andrew Dunstan
>  wrote:
>
>> here's a patch that works that way, based on Michael's code.
> Patch not attached :)
> I still have a patch half-cooked, that I can send if necessary, but
> you are on it, right?


Sorry. Here it is.

cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index 9c3f451..dfd5d8c 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -1400,3 +1400,120 @@ TypeGetTupleDesc(Oid typeoid, List *colaliases)
 
 	return tupdesc;
 }
+
+/*
+ * Extract a set of variadic argument values, types and NULL markers. This is
+ * useful for abstracting away whether or not the call has been done
+ * with an explicit VARIADIC array or a normal set of arguments, for a
+ * VARIADIC "any" function. When doing a VARIADIC call, the caller has
+ * provided one argument made of an array of keys, so deconstruct the
+ * array data before using it for the next processing.
+ * If no explicit VARIADIC call is used, just fill in the data based on
+ * all the arguments given by the caller.
+ * This function returns the number of arguments generated. In the event
+ * where the caller provided a NULL array input, return -1.
+ * nfixed is the number of non-variadic arguments to the function - these
+ * will be ignored by this function.
+ * If required by the caller, values of type "unknown" will be converted
+ * to text datums.
+ */
+int
+extract_variadic_args(FunctionCallInfo fcinfo, Datum **args,
+	  Oid **types, bool **nulls, int nfixed,
+	  bool convert_unknown_to_text)
+{
+	bool		variadic = get_fn_expr_variadic(fcinfo->flinfo);
+	Datum	   *args_res;
+	bool	   *nulls_res;
+	Oid		   *types_res;
+	int			nargs, i;
+
+	*args = NULL;
+	*types = NULL;
+	*nulls = NULL;
+
+	if (variadic)
+	{
+		ArrayType  *array_in;
+		Oid			element_type;
+		bool		typbyval;
+		char		typalign;
+		int16		typlen;
+
+		Assert(PG_NARGS() == nfixed + 1);
+
+		if (PG_ARGISNULL(nfixed))
+			return -1;
+
+		array_in = PG_GETARG_ARRAYTYPE_P(nfixed);
+		element_type = ARR_ELEMTYPE(array_in);
+
+		get_typlenbyvalalign(element_type,
+			 , , );
+		deconstruct_array(array_in, element_type, typlen, typbyval,
+		  typalign, _res, _res,
+		  );
+
+		/* All the elements of the array have the same type */
+		types_res = (Oid *) palloc0(nargs * sizeof(Oid));
+		for (i = 0; i < nargs; i++)
+			types_res[i] = element_type;
+	}
+	else
+	{
+		nargs = PG_NARGS() - nfixed;
+		Assert (nargs > 0);
+		nulls_res = (bool *) palloc0(nargs * sizeof(bool));
+		args_res = (Datum *) palloc0(nargs * sizeof(Datum));
+		types_res = (Oid *) palloc0(nargs * sizeof(Oid));
+
+		for (i = 0; i < nargs; i++)
+		{
+			nulls_res[i] = PG_ARGISNULL(i + nfixed);
+			types_res[i] = get_fn_expr_argtype(fcinfo->flinfo, i + nfixed);
+
+			if (!OidIsValid(types_res[i]))
+ereport(ERROR,
+		(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+		 errmsg("could not determine data type for argument %d",
+i + nfixed + 1)));
+
+			/*
+			 * Turn a constant (more or less literal) value that's of unknown
+			 * type into text if required . Unknowns come in as a cstring
+			 * pointer.
+			 */
+			if (convert_unknown_to_text && types_res[i] == UNKNOWNOID &&
+get_fn_expr_arg_stable(fcinfo->flinfo, i + nfixed))
+			{
+types_res[i] = TEXTOID;
+
+/* important for value, key cannot being NULL */
+if (PG_ARGISNULL(i + nfixed))
+	args_res[i] = (Datum) 0;
+else
+	args_res[i] =
+		CStringGetTextDatum(PG_GETARG_POINTER(i + nfixed));
+			}
+			else
+			{
+/* no conversion needed, just take the datum as given */
+args_res[i] = PG_GETARG_DATUM(i + nfixed);
+			}
+
+			if (!OidIsValid(types_res[i]) ||
+(convert_unknown_to_text && (types_res[i] == UNKNOWNOID)))
+ereport(ERROR,
+		(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+		 errmsg("could not determine data type for argument %d",
+i + nfixed + 1)));
+		}
+	}
+
+	/* Fill in results */
+	*args = args_res;
+	*nulls = nulls_res;
+	*types = types_res;
+
+	return nargs;
+}
diff --git a/src/include/funcapi.h b/src/include/funcapi.h
index 951af2a..0550c07 100644
--- a/src/include/funcapi.h
+++ b/src/include/funcapi.h
@@ -281,6 +281,9 @@ extern TupleTableSlot *TupleDescGetSlot(TupleDesc tupdesc);
 extern FuncCallContext *init_MultiFuncCall(PG_FUNCTION_ARGS);
 extern FuncCallContext *per_MultiFuncCall(PG_FUNCTION_ARGS);
 extern void end_MultiFuncCall(PG_FUNCTION_ARGS, FuncCallContext *funcctx);
+extern int extract_variadic_args(FunctionCallInfo fcinfo, Datum **args,
+	  Oid **types, bool **nulls, int nfixed,
+ bool convert_unknown_to_text);
 
 #define SRF_IS_FIRSTCALL() (fcinfo->flinfo->fn_extra == NULL)
 

-- 
Sent via pgsql-hackers mailing list 

[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC calls very much

[Michael Paquier]

On Mon, Oct 23, 2017 at 1:44 AM, Andrew Dunstan
 wrote:
>
>
> On 10/22/2017 12:11 PM, Andrew Dunstan wrote:
>>
>> On 10/21/2017 07:33 PM, Michael Paquier wrote:
>>> On Sun, Oct 22, 2017 at 1:43 AM, Tom Lane  wrote:
 I don't think collecting all the arguments is particularly special ---
 format() or concat() for instance could possibly use this.  You might
 need an option to say what to do with unknown.
>>> In this case, we could just use a boolean flag to decide if TEXTOID
>>> should be enforced or not.
>> A generic function is going to look a little more complicated than this,
>> though. The functions as coded assume that the function has a single
>> variadic argument. But for it to be useful generically it really needs
>> to be able to work where there are both fixed and variadic arguments (a
>> la printf style functions).
>>
>> I guess a simple way would be to make the caller tell the function where
>> the variadic arguments start, or how many to skip, something like that.

Sorry for the late reply, I was taking a long flight. Yes, that's
actually the conclusion I am reaching when looking at the code by
adding an argument to mark when the variadic arguments start. The
headers of funcapi.h and funcapi.c need also a cleanup.

> here's a patch that works that way, based on Michael's code.

Patch not attached :)
I still have a patch half-cooked, that I can send if necessary, but
you are on it, right?
-- 
Michael


-- 
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] How to determine that a TransactionId is really aborted?

[Eric Ridge]

> On Oct 22, 2017, at 1:50 PM, Peter Geoghegan  wrote:
> 
> On Sun, Oct 22, 2017 at 12:23 PM, Eric Ridge  wrote:
>> Can anyone confirm or deny that this is correct?  I feel like it is correct, 
>> but I'm no expert.
> 
> What are you going to use the code for? I think that that context is
> likely to matter here.

I'm not exactly sure yet, but I'm thinking about storing transaction ids 
externally and then regularly poking Postgres to see which ones are 
aborted-and-no-longer-considered-visible so I can clean up my external list.

eric

-- 
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] How to determine that a TransactionId is really aborted?

[Peter Geoghegan]

On Sun, Oct 22, 2017 at 12:23 PM, Eric Ridge  wrote:
> Can anyone confirm or deny that this is correct?  I feel like it is correct, 
> but I'm no expert.

What are you going to use the code for? I think that that context is
likely to matter here.

-- 
Peter Geoghegan


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] How to determine that a TransactionId is really aborted?

[Eric Ridge]

When sitting inside an extension, and given an arbitrary TransactionId, how can 
you determine that it aborted/crashed *and* that no other active transaction 
thinks it is still running?

I've tried to answer this question myself (against the 9.3 sources), and it 
seems like it's just:

{
   TransactionId oldestXmin = GetOldestXmin (false, false);
   TransactionId xid = 42;
  
   if (TransactionIdPrecedes(xid, oldestXmin) && 
  !TransactionIdDidCommit(xid) && 
  !TransactionIdIsInProgress(xid)) /* not even sure this is necessary? */
   {
  /* xid is aborted/crashed and no active transaction cares */
   }
}

Can anyone confirm or deny that this is correct?  I feel like it is correct, 
but I'm no expert.

Thanks so much for your time!

eric

-- 
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] Useless(?) asymmetry in parse_func.c

[Tom Lane]

I wrote:
> While running down loose ends in my domains-over-composite patch,
> I wondered why parse_func.c's FuncNameAsType() excludes composite
> types as possible type names.
> ...
> There might still be an argument for rejecting the case on the grounds
> that it's confusing or likely to be user error, but I'm not sure.

After studying the code more closely, I realized that there are only two
cases this restriction actually rejects, because no other cases with a
composite destination type could get past the restrictions on a
function-like cast in func_get_detail:

1. Function-style coercion of an unknown literal, eg
SELECT int8_tbl('(1,2)');

But I do not see any good argument why this should be rejected for
composite types when it works for other types.

2. Coercion of a composite type to itself, eg
SELECT int8_tbl(int8_tbl) FROM int8_tbl;
SELECT int8_tbl.int8_tbl FROM int8_tbl;

find_coercion_pathway would report that as a RELABELTYPE case, whereas
it would not recognize any other coercion to a composite type as
either RELABELTYPE or COERCEVIAIO.  (At least, not unless the user has
added such casts with CREATE CAST; in which case I think he's entitled
to expect that the system would be willing to use them.)

On the whole, probably restriction #2 is a good thing; if we were to lift
it, I think we'd start getting complaints about "why does my table seem to
have a column named after itself", rather like the complaints we got about
"why does my table seem to have a column named "text"" before we put in
the other arbitrary-seeming restriction in func_get_detail.

However, the existing code is certainly an opaque and undocumented way of
enforcing #2, plus it breaks #1 for no very good reason.  So I think we
should remove the restriction in FuncNameAsType and instead enforce #2
in a narrowly tailored way, as in the attached patch.

regards, tom lane

diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 2f2f2c7..050779b 100644
*** a/src/backend/parser/parse_func.c
--- b/src/backend/parser/parse_func.c
*** func_get_detail(List *funcname,
*** 1357,1362 
--- 1357,1367 
  		 * never supported that historically, so we can insist that people
  		 * write it as a normal cast instead.
  		 *
+ 		 * We also reject the specific case of casting a composite type to
+ 		 * itself (with RELABELTYPE).  That's not really a useful thing to do,
+ 		 * and it would likely produce user complaints about "why does my
+ 		 * table seem to have a column named after itself?"
+ 		 *
  		 * We also reject the specific case of COERCEVIAIO for a composite
  		 * source type and a string-category target type.  This is a case that
  		 * find_coercion_pathway() allows by default, but experience has shown
*** func_get_detail(List *funcname,
*** 1395,1401 
  	switch (cpathtype)
  	{
  		case COERCION_PATH_RELABELTYPE:
! 			iscoercion = true;
  			break;
  		case COERCION_PATH_COERCEVIAIO:
  			if ((sourceType == RECORDOID ||
--- 1400,1411 
  	switch (cpathtype)
  	{
  		case COERCION_PATH_RELABELTYPE:
! 			if (sourceType == targetType &&
! (sourceType == RECORDOID ||
!  ISCOMPLEX(sourceType)))
! iscoercion = false;
! 			else
! iscoercion = true;
  			break;
  		case COERCION_PATH_COERCEVIAIO:
  			if ((sourceType == RECORDOID ||
*** make_fn_arguments(ParseState *pstate,
*** 1761,1767 
   *	  convenience routine to see if a function name matches a type name
   *
   * Returns the OID of the matching type, or InvalidOid if none.  We ignore
!  * shell types and complex types.
   */
  static Oid
  FuncNameAsType(List *funcname)
--- 1771,1777 
   *	  convenience routine to see if a function name matches a type name
   *
   * Returns the OID of the matching type, or InvalidOid if none.  We ignore
!  * shell types, since casting to a shell type would be useless.
   */
  static Oid
  FuncNameAsType(List *funcname)
*** FuncNameAsType(List *funcname)
*** 1773,1780 
  	if (typtup == NULL)
  		return InvalidOid;
  
! 	if (((Form_pg_type) GETSTRUCT(typtup))->typisdefined &&
! 		!OidIsValid(typeTypeRelid(typtup)))
  		result = typeTypeId(typtup);
  	else
  		result = InvalidOid;
--- 1783,1789 
  	if (typtup == NULL)
  		return InvalidOid;
  
! 	if (((Form_pg_type) GETSTRUCT(typtup))->typisdefined)
  		result = typeTypeId(typtup);
  	else
  		result = InvalidOid;

-- 
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] Discussion on missing optimizations

[Andreas Seltenreich]

Alvaro Herrera writes:

> Andres Freund wrote:
>> Unfortunately it won't help in this specific case (no support for UNION,
>> just UNION ALL), but I thought it might be interesting to reference
>> https://medium.com/@uwdb/introducing-cosette-527898504bd6
>> here.
>
> Interesting.  I thought about a completely different approach -- use a
> fuzzer, which runs each generated query on two servers, one patched one
> not, and compare the results.  Would it be possible to tweak sqlsmith to
> do this?

I think the tweaking needed would be:

1. Log successful queries as well along with a result description.
   Maybe logging returned/affected rows is sufficent.

2. Make it avoid nondeterministic things such as joining
   pg_stat_activity or calling non-immutable functions

The second one is a bit harder and I can't think of a more elegant
solution than adding a blacklisting/whitelisting feature and let the
user do the hard work…

If these are solved though, one could make multiple runs with the same
random seed and query the logging database for differences in the result
descriptions.

regards,
Andreas


-- 
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] [sqlsmith] Failed assertion in adjust_appendrel_attrs_mutator

[Tom Lane]

Andreas Seltenreich  writes:
> testing master as of 7c981590c2, sqlsmith just triggered the following
> assertion:
> TRAP: FailedAssertion("!(!const Node*)(node))->type) == T_SubLink))", 
> File: "prepunion.c", Line: 2231)

Hmm.  adjust_appendrel_attrs() thinks it's only used after conversion
of sublinks to subplans, but this is a counterexample.  I wonder if
that assumption was ever correct?  Or maybe we need to rethink what
it does when recursing into RTE subqueries.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] [sqlsmith] Failed assertion in adjust_appendrel_attrs_mutator

[Andreas Seltenreich]

Hi,

testing master as of 7c981590c2, sqlsmith just triggered the following
assertion:

TRAP: FailedAssertion("!(!const Node*)(node))->type) == T_SubLink))", File: 
"prepunion.c", Line: 2231)

I can reproduce it on a vanilla regression database with the following
query:

--8<---cut here---start->8---
explain delete from public.road
where
EXISTS (
  select
  ref_1.tablename
from
  public.renamecolumnchild as ref_0
right join pg_catalog.pg_policies as ref_1
on (true)
where EXISTS (
  select
  public.road.name as c1,
  ref_1.with_check as c3,
  ref_2.b as c6
from
  public.itest4 as ref_2));
--8<---cut here---end--->8---

Backtrace below.

regards,
Andreas

Core was generated by `postgres: smith regression [local] DELETE
'.
Program terminated with signal SIGABRT, Aborted.
#0  0x7f70c2dacfcf in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x7f70c2dae3fa in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x564d8ce587b3 in ExceptionalCondition 
(conditionName=conditionName@entry=0x564d8cfe87b0 "!(!const 
Node*)(node))->type) == T_SubLink))", errorType=errorType@entry=0x564d8cea483d 
"FailedAssertion", fileName=fileName@entry=0x564d8cfea866 "prepunion.c", 
lineNumber=lineNumber@entry=2231) at assert.c:54
#3  0x564d8ccaf7dc in adjust_appendrel_attrs_mutator (node=0x564d91e0d2a0, 
context=0x7fff0a33caa0) at prepunion.c:2231
#4  0x564d8cc4d8ef in expression_tree_mutator (node=0x564d91e0ccd8, 
mutator=0x564d8ccaf1d0 , 
context=0x7fff0a33caa0) at nodeFuncs.c:2554
#5  0x564d8cc4d88f in expression_tree_mutator (node=0x564d91e0df90, 
mutator=mutator@entry=0x564d8ccaf1d0 , 
context=context@entry=0x7fff0a33caa0) at nodeFuncs.c:3008
#6  0x564d8ccaf29f in adjust_appendrel_attrs_mutator (node=0x564d91e0df90, 
context=0x7fff0a33caa0) at prepunion.c:2151
#7  0x564d8cc4dafb in expression_tree_mutator (node=, 
mutator=0x564d8ccaf1d0 , 
context=0x7fff0a33caa0) at nodeFuncs.c:2903
#8  0x564d8cc4e589 in range_table_mutator (rtable=, 
mutator=mutator@entry=0x564d8ccaf1d0 , 
context=context@entry=0x7fff0a33caa0, flags=flags@entry=3) at nodeFuncs.c:3146
#9  0x564d8cc4e78e in query_tree_mutator (query=0x564d91e30c20, 
query@entry=0x564d90fbe090, mutator=mutator@entry=0x564d8ccaf1d0 
, context=context@entry=0x7fff0a33caa0, 
flags=flags@entry=3) at nodeFuncs.c:3096
#10 0x564d8ccb1499 in adjust_appendrel_attrs 
(root=root@entry=0x564d910bab58, node=node@entry=0x564d90fbe090, 
nappinfos=nappinfos@entry=1, appinfos=appinfos@entry=0x7fff0a33cba8) at 
prepunion.c:1964
#11 0x564d8cca3406 in inheritance_planner (root=) at 
planner.c:1200
#12 subquery_planner (glob=glob@entry=0x564d910baac0, 
parse=parse@entry=0x564d90fbe090, parent_root=parent_root@entry=0x0, 
hasRecursion=hasRecursion@entry=0 '\000', 
tuple_fraction=tuple_fraction@entry=0) at planner.c:857
#13 0x564d8cca3dbf in standard_planner (parse=0x564d90fbe090, 
cursorOptions=256, boundParams=0x0) at planner.c:352
#14 0x564d8cd4ef5d in pg_plan_query (querytree=0x564d90fbe090, 
cursorOptions=256, boundParams=0x0) at postgres.c:807
#15 0x564d8cd4f03e in pg_plan_queries (querytrees=, 
cursorOptions=cursorOptions@entry=256, boundParams=boundParams@entry=0x0) at 
postgres.c:873
#16 0x564d8cd4f519 in exec_simple_query (query_string=0x564d8ed5c330 
"[...]") at postgres.c:1048
#17 0x564d8cd51151 in PostgresMain (argc=, 
argv=argv@entry=0x564d8ed66c50, dbname=, username=) at postgres.c:4139
#18 0x564d8ca56032 in BackendRun (port=0x564d8ed6d460) at postmaster.c:4364
#19 BackendStartup (port=0x564d8ed6d460) at postmaster.c:4036
#20 ServerLoop () at postmaster.c:1755
#21 0x564d8ccd628c in PostmasterMain (argc=3, argv=0x564d8ed3c5a0) at 
postmaster.c:1363
#22 0x564d8ca5761d in main (argc=3, argv=0x564d8ed3c5a0) at main.c:228


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC calls very much

[Andrew Dunstan]

On 10/22/2017 12:11 PM, Andrew Dunstan wrote:
>
> On 10/21/2017 07:33 PM, Michael Paquier wrote:
>> On Sun, Oct 22, 2017 at 1:43 AM, Tom Lane  wrote:
>>> I don't think collecting all the arguments is particularly special ---
>>> format() or concat() for instance could possibly use this.  You might
>>> need an option to say what to do with unknown.
>> In this case, we could just use a boolean flag to decide if TEXTOID
>> should be enforced or not.
> A generic function is going to look a little more complicated than this,
> though. The functions as coded assume that the function has a single
> variadic argument. But for it to be useful generically it really needs
> to be able to work where there are both fixed and variadic arguments (a
> la printf style functions).
>
> I guess a simple way would be to make the caller tell the function where
> the variadic arguments start, or how many to skip, something like that.
>


here's a patch that works that way, based on Michael's code.

cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



-- 
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] Allow GiST opcalsses without compress\decompres functions

[Tom Lane]

Andrey Borodin  writes:
> I was looking for a way to correctly drop compress\decompress functions from 
> opclasses.

Making a new opclass seems like a pretty grotty answer; it won't
help existing installations.

I think what you need is to undo opclasscmds.c's decision that the
dependencies should be INTERNAL.  I tried this:

regression=# ALTER OPERATOR FAMILY gist_seg_ops USING gist drop function 3 
(seg);
ERROR:  cannot drop function 3 (seg, seg) of operator family gist_seg_ops for 
access method gist: gseg_compress(internal) because operator class gist_seg_ops 
for access method gist requires it

regression=# update pg_depend set deptype = 'a' where classid = 
'pg_amproc'::regclass and objid = (select objid from pg_depend where classid = 
'pg_amproc'::regclass and refclassid = 'pg_proc'::regclass and refobjid = 
'gseg_compress(internal)'::regprocedure) and refclassid = 
'pg_opclass'::regclass and deptype = 'i';
UPDATE 1
regression=# ALTER OPERATOR FAMILY gist_seg_ops USING gist drop function 3 
(seg);
ALTER OPERATOR FAMILY

For safety, that update requires a bunch more pg_catalog
schema-qualification than I bothered with, but you get the idea.

I'm not sure if needing this hack indicates that we need more ALTER
OPERATOR CLASS/FAMILY syntax to provide a less hacky way of solving
the problem.  The idea of removing core entries of an opclass has
never come up before, and I'm not sure it would ever come up again.
If we come across another use-case maybe then would be the time to
fix it more cleanly.

regards, tom lane


-- 
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] [PATCH] Tests for reloptions

[Nikolay Shaplov]

В письме от 19 октября 2017 14:20:52 Вы написали:

> I'm hesitant to hardcode things like the number of bits in bloom, as you
> had in the original.  If I understand correctly, that number could
> change with compile options (different blocksize?), so I removed that
> part.  

#define MAX_BLOOM_LENGTH(256 * SIGNWORDBITS)

#define SIGNWORDBITS ((int) (BITS_PER_BYTE * sizeof(BloomSignatureWord)))

typedef uint16 BloomSignatureWord;

Here everything is based on uint16, and it is platform independent, as far as 
I can get.

But this is not really important now... 


> I also fixed a few spelling errors.
Thank you. I am not so good with natural languages :-)

> And pushed.
Thanx!

> Let's see what the buildfarm says about this.
It seems to me that it is quite happy about these tests :-)

> Oh, one more thing: be careful when editing parallel_schedule.  There
> are constraints on the number of entries
Oh, I did not payed attention to this issue, through it it mentioned in  
parallel_schedule comments. I've added it to the wiki 
https://wiki.postgresql.org/wiki/Regression_test_authoring So it was all 
described in one place.


> in each group; you had added a
> 20th entry after the comment that the group can only have 19.
Oups it was definitely my mistake. I should be more attentive... :-( 


-- 
Do code for fun. Can do it for money (Perl & C/C++ ~10h/week)


-- 
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] Allow GiST opcalsses without compress\decompres functions

[Andrey Borodin]

Hi, Tom!
> 20 сент. 2017 г., в 8:38, Tom Lane  написал(а):
> 
> Andrey Borodin  writes:
>> [ 0001-Allow-uncompressed-GiST-4.patch ]
> 
> Pushed, with a bit more work on the documentation and some minor
> cosmetic changes.
> 
> I did not like the fact that the new code paths added by the patch
> were untested, so I went ahead and removed the no-longer-needed
> no-op functions in the core GiST opclasses.  There's still room
> to improve the contrib opclasses, but that's much more tedious
> (you need to write an extension update script) so I didn't feel
> like messing with those now.

I was looking for a way to correctly drop compress\decompress functions from 
opclasses.
There are two cases: when the functions do something unnecessary (like 
TOASTing) and when they are just no-ops.

First case occurs, for example, in cube. Please see patch attached. There is no 
DROP DEFAULT and I'm doing
UPDATE pg_opclass SET opcdefault = FALSE WHERE opcname = 'gist_cube_ops';
I'm not sure it is correct way, but I haven't found any other workaround. Then 
I just create new default opclass without compress\decompress.

Second case, for example in seg. I have tried 
alter operator family gist_seg_ops using gist drop function 3 (seg);
It does not work: dependency between opclass and method is DEPENDENCY_INTERNAL 
and command is refused.
Should I create new opclass as with cube or, may be, I can delete functions 
from system catalog?

Best regards, Andrey Borodin.



0001-Create-cube-opclass-without-compress-decompress.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] new field for structured exception - query

[Pavel Stehule]

Hi

Currently we don't allow a access to internalquery field from PLpgSQL via
GET STACKED DIAGNOSTICS.

Do you think so has sense to allow the access to this field? The patch can
be very small.

Regards

Pavel

Re: [HACKERS] stalled post to mailing list - wrong filter?

[Pavel Stehule]

2017-10-22 9:08 GMT+02:00 Magnus Hagander :

>
>
> On Sun, Oct 22, 2017 at 9:02 AM, Pavel Stehule 
> wrote:
>
>> Hi
>>
>> I sent correct mail, that requires the approval - maybe bad filter?
>>
>
> There are some pretty restrictive filters in place on the mj2 lists in
> order to deal with cases where people send admin requests. This happens now
> and then, and will get released from the moderation queue quickly I'm sure
> :)
>
> Long term we'll be getting rid of those filters, but they're there for a
> while longer.
>

ok

Thank you

Pavel


> --
>  Magnus Hagander
>  Me: https://www.hagander.net/ 
>  Work: https://www.redpill-linpro.com/ 
>

Re: [HACKERS] stalled post to mailing list - wrong filter?

[Magnus Hagander]

On Sun, Oct 22, 2017 at 9:02 AM, Pavel Stehule 
wrote:

> Hi
>
> I sent correct mail, that requires the approval - maybe bad filter?
>

There are some pretty restrictive filters in place on the mj2 lists in
order to deal with cases where people send admin requests. This happens now
and then, and will get released from the moderation queue quickly I'm sure
:)

Long term we'll be getting rid of those filters, but they're there for a
while longer.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/ 

[HACKERS] stalled post to mailing list - wrong filter?

[Pavel Stehule]

Hi

I sent correct mail, that requires the approval - maybe bad filter?

Your message to pgsql-hackers has been delayed, and requires the approval
of the moderators, for the following reason(s):

GLOBAL ADMIN BODY:  /^\s*get\s+\S+\s+\S+\s*$/i matched "G#E#T STACKED
DIAGNOSTICS." at line number 4.

If you do not wish the message to be posted, or have other concerns,
please send a message to the list owners at the following address:
  pgsql-hackers-ow...@postgresql.org

Regards

Pavel