mridoni / gixsql

GixSQL is an ESQL preprocessor and a series of runtime libraries to enable GnuCOBOL to access PostgreSQL, ODBC, MySQL, Oracle and SQLite databases.
GNU General Public License v3.0
16 stars 8 forks source link

Add support for VARYING groups #38

Closed GitMensch closed 2 years ago

GitMensch commented 2 years ago

looks like they are supported when coded correctly and explicit, but not when defined via attribute (the preparser should change the first format to the same as the second):

     01  BUFFERA    PIC X(25000) VARYING.
     01  BUFFER.
           49 BUFFER-LEN    PIC S9(8) COMP-5.
           49 BUFFER-ARR    PIC X(250).

_Originally posted by @GitMensch in https://github.com/mridoni/gixsql/issues/69

GitMensch commented 2 years ago

Just to highlight: this is the main reason that code written against esqlOC and ocesql may not generate valid COBOL code which then raises a compiler error.

mridoni commented 2 years ago

The latest commit should implement it, if you look at TSQL005B.cbl, that's the test I used: it's identical to TSQL005A.cbl, it only replaces this declaration:

01 VCFLD SQL TYPE IS VARCHAR(100).      

with this:

01 VCFLD PIC X(100) VARYING.
GitMensch commented 2 years ago

The sample looks promising. Can you please add a REDEFINES for xyz-LENGTH as xyz-LEN and make xyz-DATA a group item for xyz-ARR?

At least from inspecting the sample I guess that both are translated to:

     01  VCFLD.
           49 VCFLD-LENGTH    PIC S9(8) COMP-5.
           49 VCFLD-DATA      PIC X(100).

while - for increased portability - I'd suggest

     01  VCFLD.
           49 VCFLD-LENGTH    PIC S9(8) COMP-5.
           49 VCFLD-LEN       REDEFINES VCFLD-LENGTH
                              PIC S9(8) COMP-5.
           49 VCFLD-DATA.
              50 VCFLD-ARR    PIC X(100).

or (better readability in my point of view)

     01  VCFLD.
           49 VCFLD-LENGTH    PIC S9(8) COMP-5.
              66 VCFLD-LEN    RENAMES VCFLD-LENGTH.
           49 VCFLD-DATA      PIC X(100).
              66 VCFLD-ARR    RENAMES VCFLD-DATA.

or

     01  VCFLD.
           49 VCFLD-LENGTH    PIC S9(8) COMP-5.
              66 VCFLD-LEN    RENAMES VCFLD-LENGTH.
           49 VCFLD-DATA.
              50 VCFLD-ARR    PIC X(100).

At least this will help with conversions from ocesql, as it uses those names.

Additional question: Can both SQL TYPE and VARYING be used on level 2-48 already or is that "open"? If already done it seems reasonable to add it to the same testscase files.

mridoni commented 2 years ago

The sample looks promising. Can you please add a REDEFINES for xyz-LENGTH as xyz-LEN and make xyz-DATA a group item for xyz-ARR?

Yes, I noticed this difference, and I was actually planning of making the two sub-member suffixes configurable in the future (through an environment variable and/or a command line parameter). For the moment they can be changed by modifying these two defines in TPESQLProcessing.cpp and SqlVar.h.

VARLEN_SUFFIX_DATA VARLEN_SUFFIX_LENGTH

If LEN and ARR, as suffixes are more common (I don't actually know, I have always met LENGTH/TEXT, but I rely on almost a single - though extensive - source, and TEXT didn't sound right, so I used DATA) I can use them as defaults, it's not a problem at this stage, obviously.

GitMensch commented 2 years ago

Configurable sounds nice, but if you do this via "RENAMES", then you can actually add as much as identifiers as you want to, possibly from multiple invocations of a new option like --varying=len,arr --varying=,text--varying=length.

For the codebases of my personal interest (ocesql and citpgsql) ARR and LEN are generated, so I'd suggest to default to this; but did not checked what happens with Oracle's VARCHAR. Speaking of this: VARCHAR on Oracle has a 2byte length prefix, LONG VARCHAR the 4byte length prefix matches .

mridoni commented 2 years ago

Configurable sounds nice, but if you do this via "RENAMES", then you can actually add as much as identifiers as you want to, possibly from multiple invocations of a new option like --varying=len,arr --varying=,text--varying=length.

Ok, I will consider this

Speaking of this: VARCHAR on Oracle has a 2byte length prefix, LONG VARCHAR the 4byte length prefix matches .

Same as above.

GitMensch commented 2 years ago

I will consider this

Thanks. In the meantime I've checked Pro*COBOL Docs: VARYING is also generated to LEN and ARR (number 3 of preprocessor for 2nd db type, it seems that is just the way to do it); it implicit defines a VARYING group if there is group (any level 01 to 48) with exactly two subfields of exact level 49 with PIC S9(4) COMP and PIC X(n) or PIC N(n). It doesn't matter if defining the field as PIC X(99999) VARYING, using the implicit definition with the group, or as a "plain" PIC X(99999). with an additional EXEC SQL VAR name IS LONG VARCHAR END-EXEC.

mridoni commented 2 years ago

The above-mentioned commit passes all tests, both with the default and custom suffix names. There was no need to change anything in the runtime libraries.

GitMensch commented 2 years ago

Looks nice; the only thing that I see "from the distance" is support for 2byte length fields (Oracle's EXEC SQL VAR name VARCHAR END-EXEC), I suggest to track this in a separate issue related to EXEC SQL VAR ; VARCHAR2 for auto-rtrim may be something to combine with that.

mridoni commented 2 years ago

The size of sub-items to store the length of variable-length-fields can be currently chosen at compile-time like this:

#ifdef USE_VARLEN_32
#define VARLEN_LENGTH_PIC       "9(05) BINARY"
#define VARLEN_LENGTH_SZ        4
#define VARLEN_LENGTH_T         uint32_t
#define VARLEN_BSWAP            COB_BSWAP_32
#else
#define VARLEN_LENGTH_PIC       "9(4) BINARY"
#define VARLEN_LENGTH_SZ        2
#define VARLEN_LENGTH_T         uint16_t
#define VARLEN_BSWAP            COB_BSWAP_16
#endif

The problem is that short/long field length indicators cannot be currently mixed, so in practice there is no difference between VARCHAR and LONG VARCHAR, provided that you define USE_VARLEN_32 (except at declaration time, where I think LONG VARCHAR is still not supported, so you have to use standard VARCHAR).

mridoni commented 2 years ago

This should be solved by the latest commit, I am attaching a revised tarball for 1.0.15dev2.

gixsql-1.0.15dev2.tar.gz

mridoni commented 2 years ago

Fixed in v1.0.15