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.

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

Load workbook does error "Cell is table column header - this value is not allowed" #1164

Open
sandraros opened this issue Nov 28, 2023 · 3 comments
Labels

Comments

@sandraros
Copy link
Collaborator

While loading a worksheet containing a table (do it via a demo program), I get the exception ZCX_EXCEL:

Cell is table column header - this value is not allowed

image

It's due to this code:

IF <fs_sheet_content> IS ASSIGNED AND <fs_sheet_content>-table_header IS NOT INITIAL AND lv_value IS NOT INITIAL.
READ TABLE <fs_sheet_content>-table->fieldcat ASSIGNING <ls_fieldcat> WITH KEY fieldname = <fs_sheet_content>-table_fieldname.
IF sy-subrc = 0.
<ls_fieldcat>-column_name = lv_value.
IF <ls_fieldcat>-column_name <> lv_value.
zcx_excel=>raise_text( 'Cell is table column header - this value is not allowed' ).
ENDIF.
ENDIF.
ENDIF.

It's due to an extra space in the Excel file, as you can see:

image

and it can be seen also via the ABAP debugger:

image

@sandraros sandraros added the bug label Nov 28, 2023
@darnoc312
Copy link
Contributor

darnoc312 commented Dec 18, 2023

Hello,

  1. Normal case (without a space at the end of lv_value): The exception would never raise even if it should because the move before the compare makes it impossible

  2. Special case like above: Even after the move the compare is unequal and raises an unwanted exception

  3. Column header should be protected from any overwriting incl. formulas or even empty values

    All can be solved with the following code:

  DATA lv_column_name TYPE zexcel_s_fieldcatalog-column_name.
  IF <fs_sheet_content> IS ASSIGNED AND <fs_sheet_content>-table_header IS NOT INITIAL.
    READ TABLE <fs_sheet_content>-table->fieldcat ASSIGNING <ls_fieldcat> WITH KEY fieldname = <fs_sheet_content>-table_fieldname.
    IF sy-subrc = 0.
      IF ip_value IS SUPPLIED.  “don’t use here lv_value is not initial
        lv_column_name = lv_value.
        IF <ls_fieldcat>-column_name <> lv_column_name.
          zcx_excel=>raise_text( 'Cell is table column header - this value is not allowed' ).
        ENDIF.
      ELSE.
        zcx_excel=>raise_text( 'Cell is table column header - this formula is not allowed' ).
      ENDIF.
    ENDIF.
  ENDIF.

image

And because of 3) the code has to move to a proper position after the main IF ip_value IS SUPPLIED.

Bernd

@nxdiez
Copy link

nxdiez commented Feb 12, 2024

Hi,
I'm facing this issue, and applied correction suggested by darnoc312, but it is not 100% correct.
If you have a column header greater than 40 characters an error is catched as en error, because column_name content is truncated to 40
image
As far as I understand by debugging, this column_name is setted when it is initial at method zcl_excel_worksheet->normalize_column_heading_texts where it takes the content of fieldcatalog scrrtext_l, but that field is 40 characters length.
Field catalog comes from zcl_excel_reader_2001=>load_worksheet_tables -> zcl_excel_worksheet=>bind_table methods.
In my opinon load_worksheet_tables by setting ls_field_catalog-column_name = ls_table_column
image
But maybe there are side effects

So I've checked deeply what is being done in "set_cell".
I think is to rewritting the content of table's fieldcatalog by setting again the field column_name and check that the length is no longer than 255. (*)
So I've modified that part of code with the following, and it seems to work fine for me.
image

IF <fs_sheet_content> IS ASSIGNED AND <fs_sheet_content>-table_header IS NOT INITIAL AND lv_value IS NOT INITIAL.
READ TABLE <fs_sheet_content>-table->fieldcat ASSIGNING <ls_fieldcat> WITH KEY fieldname = <fs_sheet_content>-table_fieldname.
IF sy-subrc = 0.
*Start modification Issue 1164
IF ip_value IS SUPPLIED.
IF strlen( lv_value ) > 255. "Maximum length for column header.
zcx_excel=>raise_text( 'Cell is table column header - this value is not allowed' ).
ENDIF.
<ls_fieldcat>-column_name = lv_value.
ELSE.
zcx_excel=>raise_text( 'Cell is table column header - this formula is not allowed' ).
ENDIF.
*End modification Issue 1164
ENDIF.
ENDIF. `

Best regards.

(*) I've found https://answers.microsoft.com/en-us/msoffice/forum/all/character-limits-for-table-headings-in-excel-2016/f23e9755-1439-48b9-900d-a815bcf12ceb#:~:text=If%20a%20cell%20has%20more,to%20255%20(with%20no%20warning) and maybe that's why fieldcatalod-Column_name is a char255 and not an String.
(**) Sorry, but I am not able to format the code ...

@darnoc312
Copy link
Contributor

Hello,

this problem is caused by using method bind_table not only in the writing process but now also in the reading process.
The feature of reading a table was added recently (Issue #1158 and pull request #1159).
I think using bind_table was the quickest way to implement this but it is not the best way.
In my opinion this should be changed in the future.

Currently this leads to the side effect that the check to protect the cell from overwriting which contains the column header
according to table's fieldcatalog-column_name is running always by reading a table instead of only by using method set_cell wrongly.
Because the setting of any sheet content cell containing table's column header and data is done twice.
bind_table called by reader's method load_worksheet_tables sets the column headers to their values and the data fields to empty values.
Then reader's method load_worksheet sets the sheet content containing column header and data again.
For the column headers this leads to the protection logic and the data fields get their final values at this time.

To your suggestions:

  1. I completely agree to your idea to use column_name instead of scrtext_l in method load_worksheet_tables.
    See issue Missing feature: reader doesn't retrieve the zcl_excel_table objects #1158 above: Point one in my first comment (not taken into account so far).
  2. If this is fixed my intended code should work properly. The values to compare then should be the same.
    Remember: The purpose of the code is to protect a column header cell from overwriting.
    Neither writing fieldcatalog nor writing sheet content with any different value is intended there.
  3. Strlen > 255: Very unlikely but another reason to use my code. The move "lv_column_name = lv_value" would
    truncate the value and the compare to fieldcatalog's column name works again.

Bernd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants