On Wed, 23 Dec 2020 at 18:46, Michael Paquier <[email protected]> wrote:
> I have begun a new thread about this point as that's a separate
> topic. I did not see other places in need of a similar cleanup:
> https://www.postgresql.org/message-id/[email protected]
Thanks. I'll look at that shortly.
> > I didn't look in detail, but it looks like if we define LOWER_NODE on
> > Windows that it might break pg_upgrade. I guess you could say it's
> > partially broken now as the behaviour there will depend on if you
> > build using Visual Studio or cygwin. We'd define LOWER_NODE on cygwin
> > but not on VS. Looks like a pg_upgrade might be problematic there
> > today.
> >
> > It feels a bit annoying to add some special case to the script to
> > maintain the status quo there. An alternative to that would be to
> > modify the .c code at #ifdef LOWER_NODE to also check we're not
> > building on VS. Neither option seems nice.
>
> Hmm. It seems that you are right here. This influences lquery
> parsing so it may be nasty and this exists since ltree is present in
> the tree (2002). I think that I would choose the update in the C code
> and remove LOWER_NODE while keeping the scripts clean, and documenting
> directly in the code why this compatibility issue exists.
> REFINT_VERBOSE is no problem, fortunately.
I ended up modifying each place in the C code where we check
LOWER_NODE. I found 2 places, one in crc32.c and another in ltree.h.
I added the same comment to both to explain why there's a check for
!defined(_MSC_VER) there. I'm not particularly happy about this code,
but I don't really see what else to do right now.
> I have tested your patch, and this is causing compilation failures for
> hstore_plpython, jsonb_plpython and ltree_plpython. So
> AddTransformModule is missing something here when compiling with
> Python.
Oh thanks for finding that. That was due to some incorrect Perl code
I'd written to add the includes from one project into another. Fixed
by:
- $p->AddIncludeDir(join(";", $pl_proj->{includes}));
+ foreach my $inc (keys %{ $pl_proj->{includes} } )
+ {
+ $p->AddIncludeDir($inc);
+ }
+
David
diff --git a/contrib/ltree/crc32.c b/contrib/ltree/crc32.c
index 8fed3346e8..b035706c05 100644
--- a/contrib/ltree/crc32.c
+++ b/contrib/ltree/crc32.c
@@ -9,7 +9,15 @@
#include "postgres.h"
-#ifdef LOWER_NODE
+/*
+ * Below we ignore the fact that LOWER_NODE is defined when compiling with
+ * MSVC. The reason for this is that earlier versions of the MSVC build
+ * scripts failed to define LOWER_NODE. More recent version of the MSVC
+ * build scripts parse makefiles which results in LOWER_NODE now being
+ * defined. We check for _MSC_VER here so as not to break pg_upgrade when
+ * upgrading from versions MSVC versions where LOWER_NODE was not defined.
+ */
+#if defined(LOWER_NODE) && !defined(_MSC_VER)
#include <ctype.h>
#define TOLOWER(x) tolower((unsigned char) (x))
#else
diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h
index dc68a0c212..8c10384503 100644
--- a/contrib/ltree/ltree.h
+++ b/contrib/ltree/ltree.h
@@ -90,7 +90,15 @@ typedef struct
#define LQL_NOT 0x10 /* level has '!' (NOT) prefix */
#define LQL_COUNT 0x20 /* level is non-'*' and has repeat
counts */
-#ifdef LOWER_NODE
+/*
+ * Below we ignore the fact that LOWER_NODE is defined when compiling with
+ * MSVC. The reason for this is that earlier versions of the MSVC build
+ * scripts failed to define LOWER_NODE. More recent version of the MSVC
+ * build scripts parse makefiles which results in LOWER_NODE now being
+ * defined. We check for _MSC_VER here so as not to break pg_upgrade when
+ * upgrading from versions MSVC versions where LOWER_NODE was not defined.
+ */
+#if defined(LOWER_NODE) && !defined(_MSC_VER)
#define FLG_CANLOOKSIGN(x) ( ( (x) & ( LQL_NOT | LVAR_ANYEND | LVAR_SUBLEXEME
) ) == 0 )
#else
#define FLG_CANLOOKSIGN(x) ( ( (x) & ( LQL_NOT | LVAR_ANYEND | LVAR_SUBLEXEME
| LVAR_INCASE ) ) == 0 )
