Hi Greg,
I was able to successfully execute all the tests of the v17-0001 patch.
But I do have a couple of additional review comments about the test code.
COMMENT - missing "any" tests
In my earlier code review (previous email) I suggested that "any" should be
added as valid option to th
Hi Greg,
I have spent some time reading this discussion thread, and doing a code review
of the latest (v17-0001) patch.
Below are my review comments; some are trivial, others not so much.
GENERAL COMMENT 1 ("any")
"any" should be included as valid option for target_server_type.
IIUC tar
Hi Michael,
Sorry I was AWOL for a couple of months. Thanks for taking the patch further,
and committing it.
Kind Regards
---
Peter Smith
Fujitsu Australia
Hello Michael,
> In short, attached is an updated version of your patch which attempts to
> solve that. I have tested this with some cplusplus stuff, and GCC for both
> versions (static_assert is available in GCC >= 6, but a manual change of c.h
> does the trick).
> I have edited the patch a b
-Original Message-
From: Andres Freund Sent: Tuesday, 3 December 2019 2:56 AM
> +StaticAssertDecl(lengthof(LockTagTypeNames) == (LOCKTAG_ADVISORY + 1),
> + "LockTagTypeNames array inconsistency");
> +
>
>These error messages strike me as somewhat unhelpful. I'd
> It seems to me that there is a good point to be consistent with the treatment
> of StaticAssertStmt and StaticAssertExpr in c.h, which have fallback
> implementations in *all* the configurations supported.
Consistency is good, but:
* That is beyond the scope for what I wanted my patch to achi
Hi Andres,
>>> As far as I can tell we should be able to use the prototype based approach
>>> in all the cases where we currently use the "negative bit-field width"
>>> approach?
>> ...
>> But I did not refactor existing code to use the new way because I was
>> fearful that there might be some
Hi Hackers.
This submission seems to have stalled.
Please forgive this post - I am unsure if the submission process expects me to
come to defence of my own patch for one last gasp, or if I am supposed to just
sit back and watch it die a slow death of a thousand cuts.
I thought this submission
From: Thomas Munro Sent: Monday, 4 November 2019 1:43
PM
> No comment on the patch but I noticed that the documentation changes don't
> build. Please make sure you can "make docs" successfully, having installed
> the documentation tools[1].
Thanks for the feedback. An updated patch which fix
From: Andres Freund Sent: Wednesday, 13 November 2019 6:01
AM
>Peter Smith:
>
> Is there a reason to not just make StaticAssertExpr and StaticAssertStmt be
> the same? I don't want to proliferate variants that users have to understand
> if there's no compelling
> need. Nor do I think do we r
From: Kyotaro Horiguchi Sent: Monday, 28 October 2019
1:26 PM
> It is missing the __cplusplus case?
My use cases for the macro are only in C code, so that's all I was interested
in at this time.
If somebody else wants to extend the patch for C++ also (and test it) they can
do.
Kind Regards,
From: Andres Freund Sent: Sunday, 27 October 2019 12:03 PM
> My proposal for this really was just to use this as a fallback for when
> static assert isn't available. Which in turn I was just suggesting because
> Tom wanted a fallback.
The patch is updated to use "extern" technique only when "_
From: Andres Freund Sent: Sunday, 27 October 2019 12:03 PM
>>Ideally, the implementation should end up calling _Static_assert()
>>somehow, so that we get the compiler's native error message.
OK. I can work on that.
>>We could do a configure check for whether _Static_assert() works at
>>file s
Dear Hackers,
We have resurrected this 2 year old "SQL statements statistics counter"
proposal from Hari.
The attached patch addresses the previous review issues.
The commit-fest entry has been moved forward to the next commit-fest
https://commitfest.postgresql.org/25/790/
Please review again,
From: Andres Freund Sent: Tuesday, 1 October 2019 3:14 AM
>I wonder if defining the fallback static assert code to something like
> extern void static_assert_func(int static_assert_failed[(condition) ? 1 :
> -1]); isn't a solution, however. I *think* that's standard C. Seems to work
> at least
From: Amit Kapila Sent: Friday, 4 October 2019 4:50 PM
>>How about I just define them both the same?
>>#define INIT_ALL_ELEMS_ZERO {0}
>>#define INIT_ALL_ELEMS_FALSE {0}
>
>I think using one define would be preferred, but you can wait and see if
>others prefer defining different macros fo
From: Tom Lane Sent: Friday, 4 October 2019 2:08 PM
>> #define INIT_ALL_ELEMS_ZERO {0}
>> #define INIT_ALL_ELEMS_FALSE {false}
>I would say that's 100% wrong. The entire point here is that it's
>memset-equivalent, and therefore writes zeroes, regardless of what the
>datatype is.
I agree i
From: Amit Kapila Sent: Friday, 4 October 2019 1:32 PM
> Also, if we want to pursue, do we want to use INIT_ALL_ZEROES for bool
>arrays as well?
FYI - In case it went unnoticed - my last patch addresses this by defining 2
macros:
#define INIT_ALL_ELEMS_ZERO {0}
#define INIT_ALL_ELEMS_FAL
-Original Message-
From: Tom Lane Sent: Thursday, 3 October 2019 1:46 AM
> Right. I think that in general it's bad practice for an initializer to not
> specify all fields/elements of the target.
> It is okay in the specific case that we're substituting for a memset(..., 0,
> ...).
> Pe
From: Amit Kapila Sent: Wednesday, 2 October 2019
9:42 AM
> I don't have any strong opinion on this, but I would mildly prefer to
> initialize boolean array with false just for the sake of readability (we
> generally initializing booleans with false).
Done. Please see attached updated patch.
From: Amit Kapila Sent: Tuesday, 1 October 2019 8:12
PM
> +1. This seems like an improvement. I can review and take this forward
> unless there are objections from others.
FYI - I created a Commitfest entry for this here:
https://commitfest.postgresql.org/25/2290/
Kind Regards
--
Peter Smi
From: Isaac Morland Sent: Tuesday, 1 October 2019
11:32 PM
>Typical Example:
>Before:
> Datum values[Natts_pg_attribute];
> bool nulls[Natts_pg_attribute];
> ...
> memset(values, 0, sizeof(values));
> memset(nulls, false, sizeof(nulls));
>A
Dear Hackers,
I have identified some OSS code which maybe can make use of C99 designated
initialisers for nulls/values arrays.
~
Background:
There are lots of tuple operations where arrays of values and flags are being
passed.
Typically these arrays are being previously initialised 0/false by
-Original Message-
From: Michael Paquier Sent: Thursday, 19 September 2019
11:08 AM
>On Wed, Sep 18, 2019 at 04:46:30PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> Postgres doesn't seem to have it, but it would be possible to define a
>> StaticAssertDecl macro that can be used at the file
-Original Message-
From: Michael Paquier Sent: Wednesday, 18 September 2019
5:40 PM
> For some of them it could help, and we could think about a better location
> for that stuff than an unused routine. The indentation of your patch is
> weird, with "git diff --check" complaining a lo
Dear Hackers,
I have identified some OSS code where more compile-time asserts could be added.
Mostly these are asserting that arrays have the necessary length to accommodate
the enums that are used to index into them.
In general the code is already commented with warnings such as:
* "If you ad
-Original Message-
From: Masahiko Sawada Sent: Thursday, 15 August 2019
7:10 PM
> BTW I've created PoC patch for cluster encryption feature. Attached patch set
> has done some items of TODO list and some of them can be used even for finer
> granularity encryption. Anyway, the implement
Greetings,
(Apologies for any naïve thoughts below. Please correct my misunderstandings)
I am trying to understand the background for the ideas proposed and/or already
decided, but it is increasingly difficult to follow.
I’ve been watching the TDE list for several months and over that time the
> -Original Message-
> From: Stephen Frost Sent: Friday, 16 August 2019 11:01 AM
> Having direct integration with a KMS would certainly be valuable, and I don't
> see a reason to deny users that option if someone would like to spend time
> implementing it- in addition to a simpler mechan
> From: Ibrar Ahmed Sent: Sunday, 18 August 2019 2:43 AM
> +1 for voice call, bruce we usually have a weekly TDE call. I will include
> you in
If you don't mind, please also add me to that TDE call list.
Thanks/Regards,
---
Peter Smith
Fujitsu Australia
On Sat, Aug 10, 2019 at 1:19 AM Tomas Vondra
wrote:
> We can of course support "forced" re-encryption, but I think it's acceptable
> if that's fairly expensive as long as it can be throttled and executed in the
> background (kinda similar to the patch to enable checksums in the background).
Hi Masahiko,
> Let me briefly explain the current design I'm thinking. The design employees
> 2-tier key architecture. That is, a database cluster has one
> master key and per-tablespace keys which are encrypted with the master key
> before storing to disk. Each tablespace keys are generated
> r
32 matches
Mail list logo
