public inbox for [email protected]
help / color / mirror / Atom feedRe: Missing docs on AT TIME ZONE precedence?
5+ messages / 3 participants
[nested] [flat]
* Re: Missing docs on AT TIME ZONE precedence?
@ 2023-11-27 21:09 Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Dunstan @ 2023-11-27 21:09 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Alvaro Herrera <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Shay Rojansky <[email protected]>; pgsql-hackers
On 2023-11-27 Mo 15:34, Tom Lane wrote:
> Alvaro Herrera <[email protected]> writes:
>> (TBH I don't think the added comments really explain the problems fully.
>> That's most likely because I don't actually understand what the problems
>> are.)
> The actual problem is that nobody has applied a cluestick to the SQL
> committee about writing an unambiguous grammar :-(. But I digress.
>
> I don't like the existing coding for more reasons than just
> underdocumentation. Global assignment of precedence is a really,
> really dangerous tool for solving ambiguous-grammar problems, because
> it can mask problems unrelated to the one you think you are solving:
> basically, it eliminates bison's complaints about grammar ambiguities
> related to the token you mark. (Commits 12b716457 and 28a61fc6c are
> relevant here.) Attaching precedence to individual productions is
> far safer, because it won't have any effect that extends beyond that
> production. You still need a precedence attached to the lookahead
> token; but I think we should try very hard to not assign a precedence
> different from IDENT's to any unreserved keywords.
>
> After a bit of fooling around I found a patch that seems to meet
> that criterion; attached.
>
>
Looks good. Perhaps the comments above the UNBOUNDED precedence setting
(esp. the first paragraph) need strengthening, with a stern injunction
to avoid different precedence for non-reserved keywords if at all possible.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Missing docs on AT TIME ZONE precedence?
@ 2023-11-28 15:27 Tom Lane <[email protected]>
parent: Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Tom Lane @ 2023-11-28 15:27 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Bruce Momjian <[email protected]>; Shay Rojansky <[email protected]>; pgsql-hackers
Andrew Dunstan <[email protected]> writes:
> Looks good. Perhaps the comments above the UNBOUNDED precedence setting
> (esp. the first paragraph) need strengthening, with a stern injunction
> to avoid different precedence for non-reserved keywords if at all possible.
OK. How about rewriting that first para like this?
* Sometimes it is necessary to assign precedence to keywords that are not
* really part of the operator hierarchy, in order to resolve grammar
* ambiguities. It's best to avoid doing so whenever possible, because such
* assignments have global effect and may hide ambiguities besides the one
* you intended to solve. (Attaching a precedence to a single rule with
* %prec is far safer and should be preferred.) If you must give precedence
* to a new keyword, try very hard to give it the same precedence as IDENT.
* If the keyword has IDENT's precedence then it clearly acts the same as
* non-keywords and other similar keywords, thus reducing the risk of
* unexpected precedence effects.
*
* We used to need to assign IDENT an explicit precedence just less than Op,
* to support target_el without AS. While that's not really necessary since
* we removed postfix operators, we continue to do so because it provides a
* reference point for a precedence level that we can assign to other
* keywords that lack a natural precedence level.
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Missing docs on AT TIME ZONE precedence?
@ 2023-11-28 15:48 Andrew Dunstan <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Dunstan @ 2023-11-28 15:48 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Bruce Momjian <[email protected]>; Shay Rojansky <[email protected]>; pgsql-hackers
On 2023-11-28 Tu 10:27, Tom Lane wrote:
> Andrew Dunstan <[email protected]> writes:
>> Looks good. Perhaps the comments above the UNBOUNDED precedence setting
>> (esp. the first paragraph) need strengthening, with a stern injunction
>> to avoid different precedence for non-reserved keywords if at all possible.
> OK. How about rewriting that first para like this?
>
> * Sometimes it is necessary to assign precedence to keywords that are not
> * really part of the operator hierarchy, in order to resolve grammar
> * ambiguities. It's best to avoid doing so whenever possible, because such
> * assignments have global effect and may hide ambiguities besides the one
> * you intended to solve. (Attaching a precedence to a single rule with
> * %prec is far safer and should be preferred.) If you must give precedence
> * to a new keyword, try very hard to give it the same precedence as IDENT.
> * If the keyword has IDENT's precedence then it clearly acts the same as
> * non-keywords and other similar keywords, thus reducing the risk of
> * unexpected precedence effects.
> *
> * We used to need to assign IDENT an explicit precedence just less than Op,
> * to support target_el without AS. While that's not really necessary since
> * we removed postfix operators, we continue to do so because it provides a
> * reference point for a precedence level that we can assign to other
> * keywords that lack a natural precedence level.
>
>
LGTM. Thanks.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Missing docs on AT TIME ZONE precedence?
@ 2023-11-28 18:34 Tom Lane <[email protected]>
parent: Andrew Dunstan <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Tom Lane @ 2023-11-28 18:34 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Bruce Momjian <[email protected]>; Shay Rojansky <[email protected]>; pgsql-hackers
Andrew Dunstan <[email protected]> writes:
> On 2023-11-28 Tu 10:27, Tom Lane wrote:
>> OK. How about rewriting that first para like this?
> LGTM. Thanks.
Thanks for reviewing. While checking things over one more time,
I noticed that there was an additional violation of this precept,
dating back to long before we understood the hazards: SET is
given its own priority, when it could perfectly well share that
of IDENT. I adjusted that and pushed.
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B.
@ 2026-03-11 13:53 Antonin Houska <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw)
Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler
(when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY()
is actually smaller than what VARSIZE_4B() expects. This patch adjusts the
VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b
structure. We assume that varlena value always starts with the length word.
The problem does not exist in the tree at the moment since the current users
of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the
compiler has no idea about the memory available. However, in an upcoming
patch, it makes sense to pass a pointer to a local variable of "varlena"
type. In such a case, the compiler warning might appear because
sizeof(varlena) is lower than sizeof(varattrib_4b).
---
src/include/varatt.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/include/varatt.h b/src/include/varatt.h
index 000bdf33b92..31063e5d4f1 100644
--- a/src/include/varatt.h
+++ b/src/include/varatt.h
@@ -207,7 +207,7 @@ typedef struct
/* VARSIZE_4B() should only be used on known-aligned data */
#define VARSIZE_4B(PTR) \
- (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF)
+ (*((const uint32 *) (PTR)) & 0x3FFFFFFF)
#define VARSIZE_1B(PTR) \
(((const varattrib_1b *) (PTR))->va_header & 0x7F)
#define VARTAG_1B_E(PTR) \
@@ -240,7 +240,7 @@ typedef struct
/* VARSIZE_4B() should only be used on known-aligned data */
#define VARSIZE_4B(PTR) \
- ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF)
+ ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF)
#define VARSIZE_1B(PTR) \
((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F)
#define VARTAG_1B_E(PTR) \
--
2.47.3
--cldir6umqisr673d
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v45-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2026-03-11 13:53 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-11-27 21:09 Re: Missing docs on AT TIME ZONE precedence? Andrew Dunstan <[email protected]>
2023-11-28 15:27 ` Tom Lane <[email protected]>
2023-11-28 15:48 ` Andrew Dunstan <[email protected]>
2023-11-28 18:34 ` Tom Lane <[email protected]>
2026-03-11 13:53 [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox