Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 15 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9154f83
initial commit
jheinri Apr 22, 2022
7b8037a
Update file-formats/tabl/type/zif_aff_indx_v1.intf.abap
jheinri Apr 25, 2022
003546d
Update file-formats/tabl/type/zif_aff_indx_v1.intf.json
jheinri Apr 25, 2022
02727ae
Update file-formats/tabl/type/zif_aff_indx_v1.intf.abap
jheinri Apr 25, 2022
6b650cb
Update file-formats/tabl/README.md
jheinri Apr 28, 2022
4d0d206
Merge branch 'main' into feature/INDX
albertmink May 11, 2022
eb769fb
Merge branch 'main' into feature/INDX
schneidermic0 Jun 1, 2022
82ba837
Several additions, FTI included
jheinri Jun 20, 2022
13e184d
Merge branch 'feature/INDX' of https://github.com/jheinri/abap-file-f…
jheinri Jun 21, 2022
bddf546
Update file-formats/tabl/type/zif_aff_indx_v1.intf.abap
jheinri Jul 8, 2022
c80bc16
delete obsolete files
jheinri Jul 8, 2022
084ab1a
Update zif_aff_indx_v1.intf.abap
jheinri Jul 11, 2022
c584f2d
Update zif_aff_indx_v1.intf.json
jheinri Jul 11, 2022
bc76e7e
Merge branch 'main' into feature/INDX
larshp Sep 24, 2022
b5a3c39
Merge branch 'main' into feature/INDX
albertmink Feb 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions file-formats/tabl/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# TABL File Format

File | Cardinality | Definition | Schema | Example
:--- | :--- | :--- | :--- | :---
`<name>.tabl.json` | 1 | *does not exist, yet* | *does not exist, yet* | *does not exist, yet*
`<name>.tabl.<indexname>.indx.json` | 0..n | [`zif_aff_indx_v1.intf.abap`](./type/zif_aff_indx_v1.intf.abap) | [`indx-v1.json`](./indx-v1.json) | [`zaff_exmpl_tabl.tabl.001.indx.json`](./examples/zaff_exmpl_tabl.tabl.001.indx.json)
jheinri marked this conversation as resolved.
Show resolved Hide resolved
109 changes: 109 additions & 0 deletions file-formats/tabl/indx-v1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
{
schneidermic0 marked this conversation as resolved.
Show resolved Hide resolved
"$comment": "This file is autogenerated, do not edit manually, see https://github.com/SAP/abap-file-formats for more information.",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://github.com/SAP/abap-file-formats/blob/main/file-formats/indx/indx-v1.json",
"title": "ABAP DDIC Secondary Index Properties",
"description": "Object type INDX",
"type": "object",
"properties": {
"formatVersion": {
"title": "ABAP File Format Version",
"description": "The ABAP file format version",
"type": "string",
"const": "1"
},
"header": {
"title": "Header",
"description": "Header",
"type": "object",
"properties": {
"description": {
"title": "Description",
"description": "Description of the ABAP object",
"type": "string",
"maxLength": 60
},
"originalLanguage": {
"title": "Original Language",
"description": "Original language of the ABAP object",
"type": "string",
"minLength": 2,
"maxLength": 2,
"pattern": "^[a-z]+$"
},
"abapLanguageVersion": {
"title": "ABAP Language Version",
"description": "ABAP language version",
"type": "string",
"enum": [
"standard",
"keyUser",
"cloudDevelopment"
],
"enumTitles": [
"Standard",
"ABAP for Key Users",
"ABAP Cloud Development"
],
"enumDescriptions": [
"Standard",
"ABAP for key user extensibility",
"ABAP cloud development"
],
"default": "standard"
}
},
"additionalProperties": false,
"required": [
"description",
"originalLanguage"
]
},
"textIndex": {
"title": "Text Index",
"description": "Standard- or text-Index",
"type": "boolean",
"default": false
},
"uniqueIndex": {
"title": "Unique",
"description": "Unique flag",
"type": "boolean",
"default": false
},
"indexFields": {
"title": "Fields",
"description": "Index fields",
"type": "array",
"uniqueItems": true,
"items": {
"title": "Index Fields",
"description": "Index fields",
"type": "object",
"properties": {
"fieldname": {
"title": "Field Name",
"description": "Field name",
"type": "string",
"maxLength": 30
},
"position": {
"title": "Field Position",
"description": "Position",
"type": "string",
"maxLength": 4,
"pattern": "^[0-9]+$"
}
},
"additionalProperties": false
}
}
},
"additionalProperties": false,
"required": [
"formatVersion",
"header",
"textIndex",
"uniqueIndex"
]
}
42 changes: 42 additions & 0 deletions file-formats/tabl/type/zif_aff_indx_v1.intf.abap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
INTERFACE zif_aff_indx_v1
PUBLIC.

TYPES:
"! <p class="shorttext">Index Fields</p>
"! Index fields
BEGIN OF ty_indexfield,
"! <p class="shorttext">Field Name</p>
"! Field name
fieldname TYPE c LENGTH 30,
jheinri marked this conversation as resolved.
Show resolved Hide resolved
"! <p class="shorttext">Field Position</p>
"! Position
position TYPE n LENGTH 4,
schneidermic0 marked this conversation as resolved.
Show resolved Hide resolved
END OF ty_indexfield,
tt_index_fields TYPE SORTED TABLE OF ty_indexfield WITH UNIQUE KEY fieldname.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


TYPES:
"! <p class="shorttext">ABAP DDIC Secondary Index Properties</p>
"! Object type INDX
BEGIN OF ty_main,
"! $required
format_version TYPE zif_aff_types_v1=>ty_format_version,
"! <p class="shorttext">Header</p>
"! Header
"! $required
header TYPE zif_aff_types_v1=>ty_header_60,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

@jheinri jheinri Apr 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"! <p class="shorttext">Text Index</p>
"! Standard- or text-Index
"! $required
"! $default 'abap_false'
text_index TYPE abap_bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"! <p class="shorttext">Unique</p>
"! Unique flag
"! $required
"! $default 'abap_false'
jheinri marked this conversation as resolved.
Show resolved Hide resolved
unique_index TYPE abap_bool,
schneidermic0 marked this conversation as resolved.
Show resolved Hide resolved
"! <p class="shorttext">Fields</p>
"! Index fields
index_fields TYPE tt_index_fields,
schneidermic0 marked this conversation as resolved.
Show resolved Hide resolved
END OF ty_main.

ENDINTERFACE.
7 changes: 7 additions & 0 deletions file-formats/tabl/type/zif_aff_indx_v1.intf.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"formatVersion": "1",
"header": {
"description": "DDIC: form based editor for secondary indexes",
jheinri marked this conversation as resolved.
Show resolved Hide resolved
"originalLanguage": "en"
}
}