public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] hex squash commit
4+ messages / 4 participants
[nested] [flat]
* [PATCH] hex squash commit
@ 2021-01-01 20:04 Bruce Momjian <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Bruce Momjian @ 2021-01-01 20:04 UTC (permalink / raw)
---
src/backend/replication/backup_manifest.c | 2 +-
src/backend/utils/adt/encode.c | 34 +------------------
src/backend/utils/adt/varlena.c | 2 +-
src/common/Makefile | 2 +-
src/common/{hex_decode.c => hex.c} | 40 ++++++++++++++++++++---
src/include/common/hex.h (new) | 18 ++++++++++
src/include/common/hex_decode.h (gone) | 16 ---------
src/include/utils/builtins.h | 3 --
src/tools/msvc/Mkvcbuild.pm | 2 +-
9 files changed, 59 insertions(+), 60 deletions(-)
diff --git a/src/backend/replication/backup_manifest.c b/src/backend/replication/backup_manifest.c
index c3f339c556..716f114d78 100644
--- a/src/backend/replication/backup_manifest.c
+++ b/src/backend/replication/backup_manifest.c
@@ -17,7 +17,7 @@
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "replication/backup_manifest.h"
-#include "utils/builtins.h"
+#include "common/hex.h"
#include "utils/json.h"
static void AppendStringToManifest(backup_manifest_info *manifest, char *s);
diff --git a/src/backend/utils/adt/encode.c b/src/backend/utils/adt/encode.c
index a6c65b1657..bca941a496 100644
--- a/src/backend/utils/adt/encode.c
+++ b/src/backend/utils/adt/encode.c
@@ -15,7 +15,7 @@
#include <ctype.h>
-#include "common/hex_decode.h"
+#include "common/hex.h"
#include "mb/pg_wchar.h"
#include "utils/builtins.h"
#include "utils/memutils.h"
@@ -141,38 +141,6 @@ binary_decode(PG_FUNCTION_ARGS)
}
-/*
- * HEX
- */
-
-static const char hextbl[] = "0123456789abcdef";
-
-uint64
-hex_encode(const char *src, size_t len, char *dst)
-{
- const char *end = src + len;
-
- while (src < end)
- {
- *dst++ = hextbl[(*src >> 4) & 0xF];
- *dst++ = hextbl[*src & 0xF];
- src++;
- }
- return (uint64) len * 2;
-}
-
-static uint64
-hex_enc_len(const char *src, size_t srclen)
-{
- return (uint64) srclen << 1;
-}
-
-static uint64
-hex_dec_len(const char *src, size_t srclen)
-{
- return (uint64) srclen >> 1;
-}
-
/*
* BASE64
*/
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 9300d19e0c..79fcdcd178 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -22,7 +22,7 @@
#include "catalog/pg_type.h"
#include "common/hashfn.h"
#include "common/int.h"
-#include "common/hex_decode.h"
+#include "common/hex.h"
#include "common/unicode_norm.h"
#include "lib/hyperloglog.h"
#include "libpq/pqformat.h"
diff --git a/src/common/Makefile b/src/common/Makefile
index f624977939..93eb27a2aa 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -58,7 +58,7 @@ OBJS_COMMON = \
file_perm.o \
file_utils.o \
hashfn.o \
- hex_decode.o \
+ hex.o \
ip.o \
jsonapi.o \
keywords.o \
diff --git a/src/common/hex_decode.c b/src/common/hex.c
similarity index 79%
rename from src/common/hex_decode.c
rename to src/common/hex.c
index 3ecdc73b5c..97f57bcc32 100644
--- a/src/common/hex_decode.c
+++ b/src/common/hex.c
@@ -1,7 +1,7 @@
/*-------------------------------------------------------------------------
*
- * hex_decode.c
- * hex decoding
+ * hex.c
+ * hex processing
*
*
* Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
@@ -9,7 +9,7 @@
*
*
* IDENTIFICATION
- * src/common/hex_decode.c
+ * src/common/hex.c
*
*-------------------------------------------------------------------------
*/
@@ -26,7 +26,7 @@
#else
#include "mb/pg_wchar.h"
#endif
-#include "common/hex_decode.h"
+#include "common/hex.h"
static const int8 hexlookup[128] = {
@@ -40,6 +40,26 @@ static const int8 hexlookup[128] = {
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
};
+/*
+ * HEX
+ */
+
+static const char hextbl[] = "0123456789abcdef";
+
+uint64
+hex_encode(const char *src, size_t len, char *dst)
+{
+ const char *end = src + len;
+
+ while (src < end)
+ {
+ *dst++ = hextbl[(*src >> 4) & 0xF];
+ *dst++ = hextbl[*src & 0xF];
+ src++;
+ }
+ return (uint64) len * 2;
+}
+
static inline char
get_hex(const char *cp)
{
@@ -104,3 +124,15 @@ hex_decode(const char *src, size_t len, char *dst)
return p - dst;
}
+
+uint64
+hex_enc_len(const char *src, size_t srclen)
+{
+ return (uint64) srclen << 1;
+}
+
+uint64
+hex_dec_len(const char *src, size_t srclen)
+{
+ return (uint64) srclen >> 1;
+}
diff --git a/src/include/common/hex.h b/src/include/common/hex.h
new file mode 100644
index 0000000000..76154b65af
--- /dev/null
+++ b/src/include/common/hex.h
@@ -0,0 +1,18 @@
+/*
+ * hex.h
+ * hex processing
+ *
+ * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/common/hex.h
+ */
+#ifndef COMMON_HEX_H
+#define COMMON_HEX_H
+
+extern uint64 hex_decode(const char *src, size_t len, char *dst);
+extern uint64 hex_encode(const char *src, size_t len, char *dst);
+extern uint64 hex_enc_len(const char *src, size_t srclen);
+extern uint64 hex_dec_len(const char *src, size_t srclen);
+
+#endif /* COMMON_HEX_H */
diff --git a/src/include/common/hex_decode.h b/src/include/common/hex_decode.h
deleted file mode 100644
index 1f99f069b2..0000000000
--- a/src/include/common/hex_decode.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/*
- * hex_decode.h
- * hex decoding
- *
- * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- * src/include/common/hex_decode.h
- */
-#ifndef COMMON_HEX_DECODE_H
-#define COMMON_HEX_DECODE_H
-
-extern uint64 hex_decode(const char *src, size_t len, char *dst);
-
-
-#endif /* COMMON_HEX_DECODE_H */
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 19271e0696..11ba6ae565 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -31,9 +31,6 @@ extern void domain_check(Datum value, bool isnull, Oid domainType,
extern int errdatatype(Oid datatypeOid);
extern int errdomainconstraint(Oid datatypeOid, const char *conname);
-/* encode.c */
-extern uint64 hex_encode(const char *src, size_t len, char *dst);
-
/* int.c */
extern int2vector *buildint2vector(const int16 *int2s, int n);
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 7f014a12c9..60b216cce0 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -121,7 +121,7 @@ sub mkvcbuild
our @pgcommonallfiles = qw(
archive.c base64.c checksum_helper.c
config_info.c controldata_utils.c d2s.c encnames.c exec.c
- f2s.c file_perm.c file_utils.c hashfn.c hex_decode.c ip.c jsonapi.c
+ f2s.c file_perm.c file_utils.c hashfn.c hex.c ip.c jsonapi.c
keywords.c kwlookup.c link-canary.c md5_common.c
pg_get_line.c pg_lzcompress.c pgfnames.c psprintf.c relpath.c rmtree.c
saslprep.c scram-common.c string.c stringinfo.c unicode_norm.c username.c
--
2.20.1
--IJpNTDwzlM2Ie8A6--
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: ANY_VALUE aggregate
@ 2023-01-18 17:01 Vik Fearing <[email protected]>
2023-01-19 14:27 ` Re: ANY_VALUE aggregate Peter Eisentraut <[email protected]>
2023-01-23 07:50 ` Re: ANY_VALUE aggregate David Rowley <[email protected]>
0 siblings, 2 replies; 4+ messages in thread
From: Vik Fearing @ 2023-01-18 17:01 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; PostgreSQL Hackers <[email protected]>
On 1/18/23 16:06, Peter Eisentraut wrote:
> On 05.12.22 21:18, Vik Fearing wrote:
>> On 12/5/22 15:57, Vik Fearing wrote:
>>> The SQL:2023 Standard defines a new aggregate named ANY_VALUE. It
>>> returns an implementation-dependent (i.e. non-deterministic) value
>>> from the rows in its group.
>>>
>>> PFA an implementation of this aggregate.
>>
>> Here is v2 of this patch. I had forgotten to update sql_features.txt.
>
> In your patch, the documentation says the definition is any_value("any")
> but the catalog definitions are any_value(anyelement). Please sort that
> out.
>
> Since the transition function is declared strict, null values don't need
> to be checked.
Thank you for the review. Attached is a new version rebased to d540a02a72.
--
Vik Fearing
Attachments:
[text/x-patch] 0001-Implement-ANY_VALUE-aggregate.v03.patch (6.2K, ../../[email protected]/2-0001-Implement-ANY_VALUE-aggregate.v03.patch)
download | inline diff:
From 9cf2c5b56ea38d3080c0cb9f8ef9e6229d8696b4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 9 Apr 2022 00:07:38 +0200
Subject: [PATCH] Implement ANY_VALUE aggregate
SQL:2023 defines an ANY_VALUE aggregate whose purpose is to emit an
implementation-dependent (i.e. non-deterministic) value from the
aggregated rows.
---
doc/src/sgml/func.sgml | 14 ++++++++++++++
src/backend/catalog/sql_features.txt | 1 +
src/backend/utils/adt/misc.c | 13 +++++++++++++
src/include/catalog/pg_aggregate.dat | 4 ++++
src/include/catalog/pg_proc.dat | 8 ++++++++
src/test/regress/expected/aggregates.out | 24 ++++++++++++++++++++++++
src/test/regress/sql/aggregates.sql | 6 ++++++
7 files changed, 70 insertions(+)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index b8dac9ef46..8ff9decfec 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19735,6 +19735,20 @@ SELECT NULLIF(value, '(none)') ...
</thead>
<tbody>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>any_value</primary>
+ </indexterm>
+ <function>any_value</function> ( <type>anyelement</type> )
+ <returnvalue><replaceable>same as input type</replaceable></returnvalue>
+ </para>
+ <para>
+ Chooses a non-deterministic value from the non-null input values.
+ </para></entry>
+ <entry>Yes</entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
diff --git a/src/backend/catalog/sql_features.txt b/src/backend/catalog/sql_features.txt
index abad216b7e..dfd3882801 100644
--- a/src/backend/catalog/sql_features.txt
+++ b/src/backend/catalog/sql_features.txt
@@ -520,6 +520,7 @@ T622 Trigonometric functions YES
T623 General logarithm functions YES
T624 Common logarithm functions YES
T625 LISTAGG NO
+T626 ANY_VALUE YES
T631 IN predicate with one list element YES
T641 Multiple column assignment NO only some syntax variants supported
T651 SQL-schema statements in SQL routines YES
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 220ddb8c01..a9251f977e 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -1041,3 +1041,16 @@ pg_get_replica_identity_index(PG_FUNCTION_ARGS)
else
PG_RETURN_NULL();
}
+
+/*
+ * Transition function for the ANY_VALUE aggregate
+ *
+ * Currently this just returns the first value, but in the future it might be
+ * able to signal to the aggregate that it does not need to be called anymore.
+ */
+Datum
+any_value_trans(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_DATUM(PG_GETARG_DATUM(0));
+}
+
diff --git a/src/include/catalog/pg_aggregate.dat b/src/include/catalog/pg_aggregate.dat
index 8c957437ea..aac60dee58 100644
--- a/src/include/catalog/pg_aggregate.dat
+++ b/src/include/catalog/pg_aggregate.dat
@@ -625,4 +625,8 @@
aggfinalfn => 'dense_rank_final', aggfinalextra => 't', aggfinalmodify => 'w',
aggmfinalmodify => 'w', aggtranstype => 'internal' },
+# any_value
+{ aggfnoid => 'any_value(anyelement)', aggtransfn => 'any_value_trans',
+ aggcombinefn => 'any_value_trans', aggtranstype => 'anyelement' },
+
]
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 86eb8e8c58..95e760440e 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -11891,4 +11891,12 @@
prorettype => 'bytea', proargtypes => 'pg_brin_minmax_multi_summary',
prosrc => 'brin_minmax_multi_summary_send' },
+{ oid => '8981', descr => 'arbitrary value from among input values',
+ proname => 'any_value', prokind => 'a', proisstrict => 'f',
+ prorettype => 'anyelement', proargtypes => 'anyelement',
+ prosrc => 'aggregate_dummy' },
+{ oid => '8982', descr => 'any_value transition function',
+ proname => 'any_value_trans', prorettype => 'anyelement', proargtypes => 'anyelement anyelement',
+ prosrc => 'any_value_trans' },
+
]
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index ae3b905331..b240ef522b 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -25,6 +25,24 @@ SELECT avg(a) AS avg_32 FROM aggtest WHERE a < 100;
32.6666666666666667
(1 row)
+SELECT any_value(v) FROM (VALUES (1)) AS v (v);
+ any_value
+-----------
+ 1
+(1 row)
+
+SELECT any_value(v) FROM (VALUES (NULL)) AS v (v);
+ any_value
+-----------
+
+(1 row)
+
+SELECT any_value(v) FROM (VALUES (array['hello', 'world'])) AS v (v);
+ any_value
+---------------
+ {hello,world}
+(1 row)
+
-- In 7.1, avg(float4) is computed using float8 arithmetic.
-- Round the result to 3 digits to avoid platform-specific results.
SELECT avg(b)::numeric(10,3) AS avg_107_943 FROM aggtest;
@@ -1910,6 +1928,12 @@ from (values ('a', 'b')) AS v(foo,bar);
a
(1 row)
+SELECT any_value(v) FILTER (WHERE v > 2) FROM (VALUES (1), (2), (3)) AS v (v);
+ any_value
+-----------
+ 3
+(1 row)
+
-- outer reference in FILTER (PostgreSQL extension)
select (select count(*)
from (values (1)) t0(inner_c))
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 514e3b2b39..4c0fd0d452 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -24,6 +24,10 @@ SELECT avg(four) AS avg_1 FROM onek;
SELECT avg(a) AS avg_32 FROM aggtest WHERE a < 100;
+SELECT any_value(v) FROM (VALUES (1)) AS v (v);
+SELECT any_value(v) FROM (VALUES (NULL)) AS v (v);
+SELECT any_value(v) FROM (VALUES (array['hello', 'world'])) AS v (v);
+
-- In 7.1, avg(float4) is computed using float8 arithmetic.
-- Round the result to 3 digits to avoid platform-specific results.
@@ -733,6 +737,8 @@ having exists (select 1 from onek b where sum(distinct a.four) = b.four);
select max(foo COLLATE "C") filter (where (bar collate "POSIX") > '0')
from (values ('a', 'b')) AS v(foo,bar);
+SELECT any_value(v) FILTER (WHERE v > 2) FROM (VALUES (1), (2), (3)) AS v (v);
+
-- outer reference in FILTER (PostgreSQL extension)
select (select count(*)
from (values (1)) t0(inner_c))
--
2.34.1
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: ANY_VALUE aggregate
2023-01-18 17:01 Re: ANY_VALUE aggregate Vik Fearing <[email protected]>
@ 2023-01-19 14:27 ` Peter Eisentraut <[email protected]>
1 sibling, 0 replies; 4+ messages in thread
From: Peter Eisentraut @ 2023-01-19 14:27 UTC (permalink / raw)
To: Vik Fearing <[email protected]>; PostgreSQL Hackers <[email protected]>
On 18.01.23 18:01, Vik Fearing wrote:
> On 1/18/23 16:06, Peter Eisentraut wrote:
>> On 05.12.22 21:18, Vik Fearing wrote:
>>> On 12/5/22 15:57, Vik Fearing wrote:
>>>> The SQL:2023 Standard defines a new aggregate named ANY_VALUE. It
>>>> returns an implementation-dependent (i.e. non-deterministic) value
>>>> from the rows in its group.
>>>>
>>>> PFA an implementation of this aggregate.
>>>
>>> Here is v2 of this patch. I had forgotten to update sql_features.txt.
>>
>> In your patch, the documentation says the definition is
>> any_value("any") but the catalog definitions are
>> any_value(anyelement). Please sort that out.
>>
>> Since the transition function is declared strict, null values don't
>> need to be checked.
>
> Thank you for the review. Attached is a new version rebased to d540a02a72.
This looks good to me now.
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: ANY_VALUE aggregate
2023-01-18 17:01 Re: ANY_VALUE aggregate Vik Fearing <[email protected]>
@ 2023-01-23 07:50 ` David Rowley <[email protected]>
1 sibling, 0 replies; 4+ messages in thread
From: David Rowley @ 2023-01-23 07:50 UTC (permalink / raw)
To: Vik Fearing <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, 19 Jan 2023 at 06:01, Vik Fearing <[email protected]> wrote:
> Thank you for the review. Attached is a new version rebased to d540a02a72.
I've only a bunch of nit-picks, personal preferences and random
thoughts to offer as a review:
1. I'd be inclined *not* to mention the possible future optimisation in:
+ * Currently this just returns the first value, but in the future it might be
+ * able to signal to the aggregate that it does not need to be called anymore.
I think it's unlikely that the transfn would "signal" such a thing. It
seems more likely if we did anything about it that nodeAgg.c would
maybe have some additional knowledge not to call that function if the
agg state already has a value. Just so we're not preempting how we
might do such a thing in the future, it seems best just to remove the
mention of it. I don't really think it serves as a good reminder that
we might want to do this one day anyway.
2. +any_value_trans(PG_FUNCTION_ARGS)
Many of transition function names end in "transfn", not "trans". I
think it's better to follow the existing (loosely followed) naming
pattern that a few aggregates seem to follow rather than invent a new
one.
3. I tend to try to copy the capitalisation of keywords from the
surrounding regression tests. I see the following breaks that.
+SELECT any_value(v) FILTER (WHERE v > 2) FROM (VALUES (1), (2), (3)) AS v (v);
(obviously, ideally, we'd always just follow the same capitalisation
of keywords everywhere in each .sql file, but we've long broken that
and the best way can do is be consistent with surrounding tests)
4. I think I'd use the word "Returns" instead of "Chooses" in:
+ Chooses a non-deterministic value from the non-null input values.
5. I've not managed to find a copy of the 2023 draft, so I'm assuming
you've got the ignoring of NULLs correct. I tried to see what other
databases do using https://www.db-fiddle.com/ . I was surprised to see
MySQL 8.0 returning NULL with:
create table a (a int, b int);
insert into a values(1,null),(1,2),(1,null);
select any_value(b) from a group by a;
I'd have expected "2" to be returned. (It gets even weirder without
the GROUP BY clause, so I'm not too hopeful any useful information can
be obtained from looking here)
I know MySQL doesn't follow the spec quite as closely as we do, so I
might not be that surprised if they didn't pay attention to the
wording when implementing this, however, I've not seen the spec, so I
can only speculate what value should be returned. Certainly not doing
any aggregation for any_value() when there is no GROUP BY seems
strange. I see they don't do the same with sum(). Perhaps this is just
a side effect of their loose standards when it came to columns in the
SELECT clause that are not in the GROUP BY clause.
6. Is it worth adding a WindowFunc test somewhere in window.sql with
an any_value(...) over (...)? Is what any_value() returns as a
WindowFunc equally as non-deterministic as when it's used as an
Aggref? Can we assume there's no guarantee that it'll return the same
value for each partition in each row? Does the spec mention anything
about that?
7. I wondered if it's worth adding a
SupportRequestOptimizeWindowClause support function for this
aggregate. I'm thinking that it might not be as likely people would
use something more specific like first_value/nth_value/last_value
instead of using any_value as a WindowFunc. Also, I'm currently
thinking that a SupportRequestWFuncMonotonic for any_value() is not
worth the dozen or so lines of code it would take to write it. I'm
assuming it would always be a MONOTONICFUNC_BOTH function. It seems
unlikely that someone would have a subquery with a WHERE clause in the
upper-level query referencing the any_value() aggregate. Thought I'd
mention both of these things anyway as someone else might think of
some good reason we should add them that I didn't think of.
David
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2023-01-23 07:50 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-01-01 20:04 [PATCH] hex squash commit Bruce Momjian <[email protected]>
2023-01-18 17:01 Re: ANY_VALUE aggregate Vik Fearing <[email protected]>
2023-01-19 14:27 ` Re: ANY_VALUE aggregate Peter Eisentraut <[email protected]>
2023-01-23 07:50 ` Re: ANY_VALUE aggregate David Rowley <[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