Support collating sequence for indexed file keys of alphanumeric class #177
Conversation
nberth
force-pushed
the
indexed-key-colseq-for-alphanum-groups
branch
from
2acff2b
to
b33b98e
Compare
nberth
force-pushed
the
indexed-key-colseq-for-alphanum-groups
branch
2 times, most recently
from
108a61b
to
57b4fbc
Compare
cobc/tree.c
Outdated
| @@ -4721,6 +4721,15 @@ validate_indexed_key_field (struct cb_file *f, struct cb_field *records, | |||
| } | |||
| k = CB_FIELD_PTR (key_ref); | |||
|
|
|||
| /* check collating sequence is not ignored */ | |||
| if (f->collating_sequence != NULL && | |||
| cbak == NULL && /* only if no alternate key is given (bit of hack for now) */ | |||
There was a problem hiding this comment.
Why not checking the alternative keys in a loop as well? Aren't the collations used for those?
What about split keys?
There was a problem hiding this comment.
Finding a way to avoid emitting "too much" warnings for the file-level collating sequence seems a little tedious here.
Split keys appear to always be handled as alphanumerics (as per the code).
nberth
force-pushed
the
indexed-key-colseq-for-alphanum-groups
branch
from
57b4fbc
to
9be8442
Compare
This comment was marked as outdated.
This comment was marked as outdated.
nberth
force-pushed
the
indexed-key-colseq-for-alphanum-groups
branch
5 times, most recently
from
8fcdfc1
to
511d25e
Compare
Before this change, only keys that were alphanumeric elementary items or numeric display were supported.
nberth
force-pushed
the
indexed-key-colseq-for-alphanum-groups
branch
from
511d25e
to
56e6811
Compare
|
Looks like this could be merged ; is anything missing here @GitMensch ? |
tests/testsuite.src/run_file.at
Outdated
| # This is, so far, only supported by the BDB backend | ||
| AT_CHECK([test "$COB_HAS_ISAM" = "db"], [0], [], [], | ||
| # Previous test "failed" --> other ISAM, skip the test | ||
| [AT_CHECK([true])], |
There was a problem hiding this comment.
Shouldn't this be XFAIL?
There was a problem hiding this comment.
I get an "UNEXPECTED PASS" when if I add an AT_XFAIL_IF([true]) here and when I configure --with-db.
I'm guessing AT_XFAIL_IF may not be a regular autotest macro that just does in-place shell-script expansion.
We could just turn the non-db case into a somewhat "artificial" expected failure with:
AT_XFAIL_IF([test "$COB_HAS_ISAM" != "db"])
AT_CHECK([test "$COB_HAS_ISAM" = "db"], [0], [], [],
# Previous test "failed" --> other ISAM, fail the test
[AT_CHECK([false])],
… but that may be a bit strange to do it like this.
Side note: the version of VBISAM I found here for testing this required to #define vbisam_off_t off_t in vbisam.h to compile properly.
cobc/tree.c
Outdated
| _("COLLATING SEQUENCE is ignored for non-alphanumeric" | ||
| " key '%s'"), k->name); |
There was a problem hiding this comment.
I think we should use a cb_tree pos and set it depending on which colseq is used - if it is from the key - use that as pos, if it is from the file use k as pos.
Maybe the name of the collation should be included in this diagnostic as well?
There was a problem hiding this comment.
Done and improved with a KEY or FILE prefix to better show what kind of key is ignored.
tests/testsuite.src/syn_file.at
Outdated
| [prog.cob:33: warning: handling of FILE COLLATING SEQUENCE is unfinished; implementation is likely to be changed | ||
| prog.cob:33: warning: handling of KEY COLLATING SEQUENCE is unfinished; implementation is likely to be changed | ||
| prog.cob:39: warning: handling of FILE COLLATING SEQUENCE is unfinished; implementation is likely to be changed | ||
| prog.cob:39: warning: handling of KEY COLLATING SEQUENCE is unfinished; implementation is likely to be changed | ||
| prog.cob:46: warning: handling of FILE COLLATING SEQUENCE is unfinished; implementation is likely to be changed | ||
| prog.cob:46: warning: handling of KEY COLLATING SEQUENCE is unfinished; implementation is likely to be changed |
There was a problem hiding this comment.
Why do we have two diagnostics on the same line?
There was a problem hiding this comment.
That was parser-related; I've included a fix.
Last time I checked VBISAM with 3.x (was a while ago), there were testsuite failures. |
That would be unrelated to a But even with a set of failures (even the NEWS file says that depending on the version of vbisam used there will be some unexpected passes and failures and will do so until we have a V-ISAM version released) this gets "only" down to working it out - we tinker the MSYS2 and MSVC builds for some testsuite expectations in GitHub CI as well - and this does help to catch any new issues that arise (and may also show some positive side effects we otherwise don't catch). |
|
I'm taking on the "minimal build" and send a PR for that (this should pass here in any case) --> #184 and would let the vbisam part optional to you. |
Sure, but you mentionned |
nberth
force-pushed
the
indexed-key-colseq-for-alphanum-groups
branch
from
2a365a5
to
0be00c2
Compare
Before this change, only keys that were alphanumeric elementary items or numeric display were supported.