[LIMU INDX] File format for DDIC secondary indexes #354

jheinri · 2022-04-22T10:41:24Z

AFF file format for ABAP DDIC secondary indexes

cla-assistant · 2022-04-22T10:41:40Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ albertmink
✅ larshp
✅ schneidermic0
❌ jheinri
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

schneidermic0

Thanks @jheinri for your contribution 👍

Please see my questions and comments

schneidermic0 · 2022-04-22T14:51:22Z

file-formats/tabl/type/zif_aff_indx_v1.intf.abap

+      "! <p class="shorttext">Header</p>
+      "! Header
+      "! $required
+      header         TYPE zif_aff_types_v1=>ty_header_60,


Since we have a LIMU object here, we most probably won't need a header. The original_language and the abap_language_version is derived from the TABL object, isn't it?

I think we can just add a description field instead of the complete header. What do you think?

For REPS & FUNC, we use the header zif_aff_types_v1=>ty_header_only_description, a header for sub objects which consists only of the field description. Should this be kept the same?

The original language can be derived from the table. That is not necessary true for the abap_language_version. There are still discussion going on, but currently we must assume that extension indexes (R3TR XINX) can have an abap_language_version independent from the table.
So far we assume that the file format for LIMU INDX and R3TR XINX is the same. If this is still true this would mean that we leave the header field as it is, correct?

I would suggest to have different formats (wrt the header) for LIMU INDX and R3TR XINX

If R3TR XINX supports only ABAP language version standard and cloudDevelopment you should use zif_aff_types_v1=>ty_header_60_cloud as type for field header.

schneidermic0 · 2022-04-22T14:55:42Z

file-formats/tabl/type/zif_aff_indx_v1.intf.abap

+      "! $default 'abap_false'
+      text_index     TYPE abap_bool,


Does it make sense to change this field to an enumeration with the values like standard and textIndex (or similar)? This allows you to open it up for other kind of indexes in the future.

If you want to keep it as boolean I think you can remove the default false because this is anyhow the standard.

That is in principle a good idea. The question is how this will look afterwards on the GUI. So far we are much in favor of a checkbox which only must be selected if the developer actually wants a text index (should be the 1% case). I guess if the change this to an enumeration we cannot have a checkbox, right?
The question is also related to the support of old-style fulltext indexes. That would require the ability to enable and disable certain fields based on the index type. I think so far we said that this is not possible.

It would become a drop-down with both options. The "old-style fulltext indexes" would be a third option, wouldn't it?

For the ABAP file formats it is most probably helpful to specify the full object type even if we use/support just a subset in ADT in a first version.

schneidermic0 · 2022-04-22T14:56:24Z

file-formats/tabl/type/zif_aff_indx_v1.intf.abap

+      "! Position
+      position  TYPE n LENGTH 4,
+    END OF ty_indexfield,
+    tt_index_fields TYPE SORTED TABLE OF ty_indexfield WITH UNIQUE KEY fieldname.


It would be great if we could rename the type to ty_index_fields

Suggested change

tt_index_fields TYPE SORTED TABLE OF ty_indexfield WITH UNIQUE KEY fieldname.

ty_index_fields TYPE SORTED TABLE OF ty_indexfield WITH UNIQUE KEY fieldname.

That would be at least against our DDIC coding conventions. We start simple type definitions with ty_, table types with tt_. If you have other guidelines and it is important for you I have no problem if we change this, but from our perspective this looks ok as it is.

I discussed this with the team. We would prefer to stick only to one option within this repository. I.e., types shall start with ty_.

See also #357

file-formats/tabl/type/zif_aff_indx_v1.intf.abap

file-formats/tabl/type/zif_aff_indx_v1.intf.json

file-formats/tabl/type/zif_aff_indx_v1.intf.abap

file-formats/tabl/README.md

larshp · 2022-04-22T15:13:00Z

and again, why not just make this part of TABL, it is logically part of TABL, the implementation is part of TABL?

ensuring consistency across multiple files is more work, why not just make it consistent by design?

schneidermic0 · 2022-04-25T07:55:41Z

Independent how we solve it (as part of the *.tabl.json-file or as dedicated file(s)) I would prefer to keep it also as close as possible how it is/will be provided in the development environment.

larshp · 2022-04-25T08:11:13Z

in a typical development environment, I think its pretty common to keep the table definition and index definitions close

eg. CREATE TABLE and CREATE INDEX as part of the same schema file, or else it gets difficult to keep track of the dependencies

same problem here, just not a normal SQL schema language

Co-authored-by: Michael Schneider <[email protected]>

schneidermic0 · 2022-04-26T15:10:32Z

I just want to summarize which options I see to cover indexes of TABL:

a) The indexes will be part of this file t100.tabl.json file
b) The indexes will be part of TABLs source file. The source file contains the definition of the DB table/structure and the index definitions as source code. The source code for table definitinions is known from ADT
c) Indexes will get one dedicated file (e.g. t100.tabl.indexes.json) containing all indexes of this table
d) Each index gets a dedicated file (e.g. t100.tabl.i01.indx.json) containing the content of this single index

I guess it would help taking the whole file format of TABL into account

Furthermore, extension indexes can be also created as dedicated transport object (R3TR XINX) which represents one index.

larshp · 2022-04-26T15:15:18Z

I think "b)" has more sub-options, as the source code for table definitions, I dont think it covers POOL/CLUSTER tables?

Co-authored-by: Michael Schneider <[email protected]>

- unique secondary index on fields table defined - no_db_index field added - fields for full text indexes added

…ormats into feature/INDX

file-formats/tabl/type/zif_aff_indx_v1.intf.abap~

schneidermic0

Thanks for your update, @jheinri.

Please see my and @larshp's comments and questions.

file-formats/tabl/type/zif_aff_indx_v1.intf.abap

schneidermic0 · 2022-06-22T12:21:58Z

file-formats/tabl/type/zif_aff_indx_v1.intf.abap

+      "! <p class="shorttext">Header</p>
+      "! Header
+      "! $required
+      header                TYPE if_aff_types_v1=>ty_header_60,


Do we need a full-blown header? I guess the object needs only a description. Therefore, you might want to change the type to zif_aff_types_v1=>ty_header_only_description

Suggested change

header TYPE if_aff_types_v1=>ty_header_60,

header TYPE zif_aff_types_v1=>ty_header_only_description,

As discussed, we need a full-blown header with abap_language_version. No change here.

I agree, for R3TR XINX, you will need it.
But for LIMU objects, I expect the ABAP language version and the original language are specified for the table.

I would create different headers for R3TR XINX and LIMU XINX

Yes, we can do that. Given that we have then two different interfaces (IF_AFF_INDX, IF_AFF_XINX) but just one editor for both objects - which interface/JSON should we take for the editor? Does it matter?

file-formats/tabl/type/zif_aff_indx_v1.intf.abap~

file-formats/tabl/indx-v1.json

schneidermic0

Two additional comments related to full-text indexes. I think I missed to add them in my previous review.

file-formats/tabl/type/zif_aff_indx_v1.intf.abap

Co-authored-by: Michael Schneider <[email protected]>

delete obsolete files

Distinguish between token index and fulltext index, separate structure for fulltext index properties, property "no_db_index" added

schneidermic0 · 2023-07-31T06:47:36Z

I close this PR. No updates for more than one year.

initial commit

9154f83

AFF file format for ABAP DDIC secondary indexes

schneidermic0 changed the title ~~initial commit~~ [LIMU INDX] File format for DDIC secondary indexes Apr 22, 2022

schneidermic0 self-requested a review April 22, 2022 13:05

schneidermic0 requested changes Apr 22, 2022

View reviewed changes

jheinri and others added 3 commits April 25, 2022 11:52

Update file-formats/tabl/type/zif_aff_indx_v1.intf.abap

7b8037a

Co-authored-by: Michael Schneider <[email protected]>

Update file-formats/tabl/type/zif_aff_indx_v1.intf.json

003546d

Co-authored-by: Michael Schneider <[email protected]>

Update file-formats/tabl/type/zif_aff_indx_v1.intf.abap

02727ae

Co-authored-by: Michael Schneider <[email protected]>

schneidermic0 mentioned this pull request Apr 25, 2022

2022.04b Iteration Plan #358

Closed

Update file-formats/tabl/README.md

6b650cb

Co-authored-by: Michael Schneider <[email protected]>

schneidermic0 mentioned this pull request May 9, 2022

2022.05a Iteration Plan #363

Closed

Merge branch 'main' into feature/INDX

4d0d206

schneidermic0 mentioned this pull request May 25, 2022

2022.05b Iteration Plan #366

Closed

schneidermic0 and others added 3 commits June 1, 2022 12:38

Merge branch 'main' into feature/INDX

eb769fb

Several additions, FTI included

82ba837

- unique secondary index on fields table defined - no_db_index field added - fields for full text indexes added

Merge branch 'feature/INDX' of https://github.com/jheinri/abap-file-f…

13e184d

…ormats into feature/INDX

larshp reviewed Jun 21, 2022

View reviewed changes

file-formats/tabl/type/zif_aff_indx_v1.intf.abap~ Outdated Show resolved Hide resolved

schneidermic0 requested changes Jun 22, 2022

View reviewed changes

schneidermic0 mentioned this pull request Jun 22, 2022

2022.06 Iteration Plan #368

Closed

schneidermic0 marked this pull request as draft June 30, 2022 06:25

schneidermic0 requested changes Jul 7, 2022

View reviewed changes

file-formats/tabl/type/zif_aff_indx_v1.intf.abap Outdated Show resolved Hide resolved

file-formats/tabl/type/zif_aff_indx_v1.intf.abap Outdated Show resolved Hide resolved

jheinri and others added 4 commits July 8, 2022 09:53

Update file-formats/tabl/type/zif_aff_indx_v1.intf.abap

bddf546

Co-authored-by: Michael Schneider <[email protected]>

delete obsolete files

c80bc16

delete obsolete files

Update zif_aff_indx_v1.intf.abap

084ab1a

Distinguish between token index and fulltext index, separate structure for fulltext index properties, property "no_db_index" added

Update zif_aff_indx_v1.intf.json

c584f2d

schneidermic0 mentioned this pull request Aug 3, 2022

Arrays/internal tables, sorting #176

Open

6 tasks

larshp and others added 2 commits September 24, 2022 16:52

Merge branch 'main' into feature/INDX

bc76e7e

Merge branch 'main' into feature/INDX

b5a3c39

schneidermic0 closed this Jul 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LIMU INDX] File format for DDIC secondary indexes #354

[LIMU INDX] File format for DDIC secondary indexes #354

jheinri commented Apr 22, 2022

cla-assistant bot commented Apr 22, 2022 •

edited

Loading

schneidermic0 left a comment

schneidermic0 Apr 22, 2022

wurzka Apr 25, 2022

jheinri Apr 25, 2022 •

edited

Loading

schneidermic0 Jun 1, 2022

schneidermic0 Jun 1, 2022

schneidermic0 Apr 22, 2022

jheinri Apr 25, 2022

schneidermic0 Apr 25, 2022

schneidermic0 Apr 22, 2022

jheinri Apr 25, 2022

schneidermic0 Jun 1, 2022

larshp commented Apr 22, 2022 •

edited

Loading

schneidermic0 commented Apr 25, 2022

larshp commented Apr 25, 2022

schneidermic0 commented Apr 26, 2022

larshp commented Apr 26, 2022

schneidermic0 left a comment

schneidermic0 Jun 22, 2022

jheinri Jul 8, 2022

schneidermic0 Jul 8, 2022

jheinri Jul 8, 2022

schneidermic0 left a comment

schneidermic0 commented Jul 31, 2023

	tt_index_fields TYPE SORTED TABLE OF ty_indexfield WITH UNIQUE KEY fieldname.
	ty_index_fields TYPE SORTED TABLE OF ty_indexfield WITH UNIQUE KEY fieldname.

	header TYPE if_aff_types_v1=>ty_header_60,
	header TYPE zif_aff_types_v1=>ty_header_only_description,

[LIMU INDX] File format for DDIC secondary indexes #354

[LIMU INDX] File format for DDIC secondary indexes #354

Conversation

jheinri commented Apr 22, 2022

cla-assistant bot commented Apr 22, 2022 • edited Loading

schneidermic0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jheinri Apr 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larshp commented Apr 22, 2022 • edited Loading

schneidermic0 commented Apr 25, 2022

larshp commented Apr 25, 2022

schneidermic0 commented Apr 26, 2022

larshp commented Apr 26, 2022

schneidermic0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schneidermic0 left a comment

Choose a reason for hiding this comment

schneidermic0 commented Jul 31, 2023

cla-assistant bot commented Apr 22, 2022 •

edited

Loading

jheinri Apr 25, 2022 •

edited

Loading

larshp commented Apr 22, 2022 •

edited

Loading