pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
[PATCH] Add some useful asserts into View Options macroses
5+ messages / 3 participants
[nested] [flat]

* [PATCH] Add some useful asserts into View Options macroses
@ 2019-10-05 21:23  Nikolay Shaplov <dhyan@nataraj.su>
  0 siblings, 1 reply; 5+ messages in thread

From: Nikolay Shaplov @ 2019-10-05 21:23 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>; Nikolay Shaplov <dhyan@nataraj.su>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Dent John <denty@qqdd.eu>; Iwata, Aya <iwata.aya@jp.fujitsu.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Thomas Munro <thomas.munro@gmail.com>

This thread is a follow up to the thread https://www.postgresql.org/message-id/2620882.s52SJui4ql@x200m where I've been trying to remove StdRdOptions 
structure and replace it with unique structure for each relation kind.

I've decided to split that patch into smaller parts.

This part adds some asserts to ViewOptions macroses.
Since an option pointer there is converted into (ViewOptions *) it would be 
really good to make sure that this macros is called in proper context, and we 
do the convertation properly. At least when running tests with asserts turned 
on.

-- 
Software Developer: https://www.upwork.com/freelancers/~014a87e140ff02c0da
Body-oriented Therapist: https://vk.com/nataraj_rebalancing  (Russian)

Attachments:

  [text/x-patch] some-asserts-for-view-options-macroses_v1.diff (2.0K, ../../3634983.eHpMQ1mJnI@x200m/2-some-asserts-for-view-options-macroses_v1.diff)
  download | inline diff:
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index a5cf804..b24fb1d 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -351,9 +351,10 @@ typedef struct ViewOptions
  *		Returns whether the relation is security view, or not.  Note multiple
  *		eval of argument!
  */
-#define RelationIsSecurityView(relation)	\
-	((relation)->rd_options ?				\
-	 ((ViewOptions *) (relation)->rd_options)->security_barrier : false)
+#define RelationIsSecurityView(relation)									\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW),				\
+	 ((relation)->rd_options ?												\
+	  ((ViewOptions *) (relation)->rd_options)->security_barrier : false))
 
 /*
  * RelationHasCheckOption
@@ -361,9 +362,10 @@ typedef struct ViewOptions
  *		or the cascaded check option.  Note multiple eval of argument!
  */
 #define RelationHasCheckOption(relation)									\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW),				\
 	((relation)->rd_options &&												\
 	 ((ViewOptions *) (relation)->rd_options)->check_option !=				\
-	 VIEW_OPTION_CHECK_OPTION_NOT_SET)
+	 VIEW_OPTION_CHECK_OPTION_NOT_SET))
 
 /*
  * RelationHasLocalCheckOption
@@ -371,9 +373,10 @@ typedef struct ViewOptions
  *		option.  Note multiple eval of argument!
  */
 #define RelationHasLocalCheckOption(relation)								\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW),				\
 	((relation)->rd_options &&												\
 	 ((ViewOptions *) (relation)->rd_options)->check_option ==				\
-	 VIEW_OPTION_CHECK_OPTION_LOCAL)
+	 VIEW_OPTION_CHECK_OPTION_LOCAL))
 
 /*
  * RelationHasCascadedCheckOption
@@ -381,9 +384,10 @@ typedef struct ViewOptions
  *		option.  Note multiple eval of argument!
  */
 #define RelationHasCascadedCheckOption(relation)							\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW),				\
 	((relation)->rd_options &&												\
 	 ((ViewOptions *) (relation)->rd_options)->check_option ==				\
-	  VIEW_OPTION_CHECK_OPTION_CASCADED)
+	  VIEW_OPTION_CHECK_OPTION_CASCADED))
 
 /*
  * RelationIsValid

  [application/pgp-signature] signature.asc (487B, ../../3634983.eHpMQ1mJnI@x200m/3-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [PATCH] Add some useful asserts into View Options macroses
@ 2019-10-07 16:59  Robert Haas <robertmhaas@gmail.com>
  parent: Nikolay Shaplov <dhyan@nataraj.su>
  0 siblings, 1 reply; 5+ messages in thread

From: Robert Haas @ 2019-10-07 16:59 UTC (permalink / raw)
  To: Nikolay Shaplov <dhyan@nataraj.su>; +Cc: Michael Paquier <michael@paquier.xyz>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Dent John <denty@qqdd.eu>; Iwata, Aya <iwata.aya@jp.fujitsu.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Thomas Munro <thomas.munro@gmail.com>

On Sat, Oct 5, 2019 at 5:23 PM Nikolay Shaplov <dhyan@nataraj.su> wrote:
> This thread is a follow up to the thread https://www.postgresql.org/message-id/2620882.s52SJui4ql@x200m where I've been trying to remove StdRdOptions
> structure and replace it with unique structure for each relation kind.
>
> I've decided to split that patch into smaller parts.
>
> This part adds some asserts to ViewOptions macroses.
> Since an option pointer there is converted into (ViewOptions *) it would be
> really good to make sure that this macros is called in proper context, and we
> do the convertation properly. At least when running tests with asserts turned
> on.

Seems like a good idea.  Should we try to do something similar for the
macros in that header file that cast to StdRdOptions?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [PATCH] Add some useful asserts into View Options macroses
@ 2019-10-08 10:44  Nikolay Shaplov <dhyan@nataraj.su>
  parent: Robert Haas <robertmhaas@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Nikolay Shaplov @ 2019-10-08 10:44 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Dent John <denty@qqdd.eu>; Iwata, Aya <iwata.aya@jp.fujitsu.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Thomas Munro <thomas.munro@gmail.com>

В письме от понедельник, 7 октября 2019 г. 12:59:27 MSK пользователь Robert 
Haas написал:

> > This thread is a follow up to the thread
> > https://www.postgresql.org/message-id/2620882.s52SJui4ql@x200m where I've
> > been trying to remove StdRdOptions structure and replace it with unique
> > structure for each relation kind.
> > 
> > I've decided to split that patch into smaller parts.
> > 
> > This part adds some asserts to ViewOptions macroses.
> > Since an option pointer there is converted into (ViewOptions *) it would
> > be
> > really good to make sure that this macros is called in proper context, and
> > we do the convertation properly. At least when running tests with asserts
> > turned on.
> Seems like a good idea.  Should we try to do something similar for the
> macros in that header file that cast to StdRdOptions?

That would not be as easy as for ViewOptions. For example as for the current 
master code, fillfactor from StdRdOptions is used in Toast, Heap, Hash index, 
nbtree index, and spgist index. This will make RelationGetFillFactor macros a 
bit complicated for example.

Now I have patches that limits usage of StdRdOptions to Heap and Toast.

When StdRdOptions is not that widely used, we whould be able to add asserts 
for it, it will not make the code too complex.

So I would suggest to do ViewOptions asserts now, and keep dealing with 
StdRdOptions for later. When we are finished with my current patches, I will 
take care about it.

-- 
Software Developer: https://www.upwork.com/freelancers/~014a87e140ff02c0da
Body-oriented Therapist: https://vk.com/nataraj_rebalancing  (Russian)





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [PATCH] Add some useful asserts into View Options macroses
@ 2019-11-01 12:29  Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  parent: Nikolay Shaplov <dhyan@nataraj.su>
  0 siblings, 1 reply; 5+ messages in thread

From: Peter Eisentraut @ 2019-11-01 12:29 UTC (permalink / raw)
  To: Nikolay Shaplov <dhyan@nataraj.su>; Robert Haas <robertmhaas@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Dent John <denty@qqdd.eu>; Iwata, Aya <iwata.aya@jp.fujitsu.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Thomas Munro <thomas.munro@gmail.com>

On 2019-10-08 12:44, Nikolay Shaplov wrote:
> В письме от понедельник, 7 октября 2019 г. 12:59:27 MSK пользователь Robert
> Haas написал:
> 
>>> This thread is a follow up to the thread
>>> https://www.postgresql.org/message-id/2620882.s52SJui4ql@x200m where I've
>>> been trying to remove StdRdOptions structure and replace it with unique
>>> structure for each relation kind.
>>>
>>> I've decided to split that patch into smaller parts.
>>>
>>> This part adds some asserts to ViewOptions macroses.
>>> Since an option pointer there is converted into (ViewOptions *) it would
>>> be
>>> really good to make sure that this macros is called in proper context, and
>>> we do the convertation properly. At least when running tests with asserts
>>> turned on.

Committed.

I simplified the parentheses by one level from your patch.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [PATCH] Add some useful asserts into View Options macroses
@ 2019-11-08 17:20  Nikolay Shaplov <dhyan@nataraj.su>
  parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  0 siblings, 0 replies; 5+ messages in thread

From: Nikolay Shaplov @ 2019-11-08 17:20 UTC (permalink / raw)
  To: pgsql-hackers@lists.postgresql.org; +Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; Robert Haas <robertmhaas@gmail.com>; Michael Paquier <michael@paquier.xyz>; Tomas Vondra <tomas.vondra@2ndquadrant.com>; Dent John <denty@qqdd.eu>; Iwata, Aya <iwata.aya@jp.fujitsu.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; Dmitry Dolgov <9erthalion6@gmail.com>; Thomas Munro <thomas.munro@gmail.com>

В письме от пятница, 1 ноября 2019 г. 13:29:58 MSK пользователь Peter 
Eisentraut написал:
 
> Committed.
> 
> I simplified the parentheses by one level from your patch.
Thank you!

-- 
Software Developer: https://www.upwork.com/freelancers/~014a87e140ff02c0da
Body-oriented Therapist: https://vk.com/nataraj_rebalancing  (Russian)

^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2019-11-08 17:20 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-10-05 21:23 [PATCH] Add some useful asserts into View Options macroses Nikolay Shaplov <dhyan@nataraj.su>
2019-10-07 16:59 ` Robert Haas <robertmhaas@gmail.com>
2019-10-08 10:44   ` Nikolay Shaplov <dhyan@nataraj.su>
2019-11-01 12:29     ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-11-08 17:20       ` Nikolay Shaplov <dhyan@nataraj.su>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox