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

SELECT SINGLE, vs SELECT ENDSELECT, vs SELECT UP TO 1 ROWS ENDSELECT #179

Open
larshp opened this issue Feb 8, 2021 · 18 comments
Open

SELECT SINGLE, vs SELECT ENDSELECT, vs SELECT UP TO 1 ROWS ENDSELECT #179

larshp opened this issue Feb 8, 2021 · 18 comments

Comments

@larshp
Copy link
Contributor

larshp commented Feb 8, 2021

What would be the preferred way of selecting a row from the database? SELECT SINGLE, vs SELECT ENDSELECT, vs SELECT UP TO 1 ROWS ENDSELECT

Personally I use SELECT SINGLE to select one row from the database, as its the shortest, and avoids opening a database cursor. I always use SELECT SINGLE, no matter if the full primary key is specified or not.

If ORDER BY is needed, then one of the ENDSELECT or SELECT INTO TABLE is required
If PACKAGE SIZE is needed, then one of the ENDSELECT approaches are required

some references,
https://blogs.sap.com/2016/06/11/select-single-vs-select-up-to-1-rows/
https://blogs.sap.com/2015/03/11/selecting-one-line-from-an-database-table/

@nununo
Copy link

nununo commented Feb 9, 2021

I also prefer SELECT SINGLE always. I always wondered why people keep telling me to use SELECT ENDSELECT when the full primary key is not provided. Maybe it's just for aesthetic reasons? To differentiate? Which doesn't really make sense to me. Unless someone gives me a good reason against it I'll keep using SELECT SINGLE. Except on those exceptions you mentioned, of course.

@fabianlupa
Copy link
Contributor

fabianlupa commented Feb 9, 2021

Imo the only alternative to SELECT SINGLE when the full key is not specified is to do a SELECT ... INTO TABLE UP TO 1 ROWS ORDER BY x and then use the first row in the internal table, never SELECT ... (EXIT) ENDS ELECT.
Order by should mostly be required though especially on HANA.

@StefanButscher
Copy link
Contributor

From the perspective of readability SELECT SINGLE is best. Yet, as perfromance is always a topic with SQL, it sometimes pays to experiment and double check. In case the best performing SQL statement is odd to read, it should be wrapped in a method.

@sdfraga
Copy link

sdfraga commented Feb 9, 2021

I prefer SELECT SINGLE also for the reasons already mentioned.
If I only need to check if the entry exists I am now forcing myself to use:
https://help.sap.com/doc/abapdocu_751_index_htm/7.51/en-us/abensql_expr_literal_abexa.htm

@JozsefSzikszai
Copy link

I always wondered why people keep telling me to use SELECT ENDSELECT when the full primary key is not provided. Maybe it's just for aesthetic reasons? To differentiate? .

There was a myth many-many years ago, that SELECT SINGLE uses the primary key (i. e. it is hardcoded to do so), so if you use SELECT SINGLE and don't provide the (full) primary key, SAP still wants to find the respecitve entry by the primary key, so it will result bad performance. As told it is a myth (=fake news in today's terminology), which was spreading around for years without any evidence.

@marianfoo
Copy link

I also vote for SELECT SINGLE because of readability

@Jelena-P
Copy link

Jelena-P commented Dec 7, 2022

I'm relieved to see so many ABAP luminaries voting for SELECT SINGLE. I also tend to use it, whether I have full key or not. If I don't have full key and it's important which record is returned, then I'll use UP TO 1 ROWS and ORDER BY. Otherwise I just don't care which record SELECT SINGLE returns. But some of the code quality tools (or reviewers?) sometimes have an issue with that.

I agree that SELECT SINGLE is better for readability and other options should be recommended only when actually needed. Maybe a side issue is that some (many?) developers just don't know one from the other though.

@se38
Copy link
Contributor

se38 commented Dec 8, 2022

Okay, then I'm the only one preferring UP TO 1 ROWS. To document, that the primary key is not fully supplied and you may have unpredicted results. That's also the reason I have activated the ATC check. It results into "Think twice, are you really sure or do you just have forgotten a key field in the where clause?". Saved my bud a couple of times already.

@larshp
Copy link
Contributor Author

larshp commented Dec 8, 2022

documenting that its not the full key, there is a pseudo comment for code inspector(and a pragma)

both SELECT SINGLE and UP TO 1 ROWS have the possibility to specify full or partial primary key(?)

@BiberM
Copy link

BiberM commented Dec 9, 2022

In my opinion the reason is the language restriction to SINGLE: it cannot be combined with ORDER BY. Using it for something other than the complete primary key you either just want to Check for existence or you better really know what you are doing. Forcing everyone to use SINGLE in my opinion leads to more errors when Developers use pragmas to make ATC quiet instead of thinking.

@kjetil-kilhavn
Copy link
Contributor

kjetil-kilhavn commented May 11, 2023

I try to use SELECT SINGLE only when the full key is specified. Like @se38 wrote, that is one way of making the reader aware that I have specified the full key.

If I want only one entry when the key isn't fully specified, then the code (or comments) should explain why I am only interested in one entry. Sometimes it is because there is a foreign key that is unique. But is it... Assumptions should be verified, so if I expect at most one matching entry there is no need to use UP TO 1 ROWS either. Instead:

SELECT
  FROM some_table_which_should_have_at_most_one_valid_row_at_any_given_date
  FIELDS *
  WHERE endda >= @some_date
    AND begda <= @some_date
ENDSELECT.
IF sy-subrc <> 0.
  RAISE EXCEPTION TYPE z_not_found
ELSEIF sy-dbcnt > 1.
  RAISE EXCEPTION TYPE z_more_than_one_valid_entry.
ENDIF.

I would possibly just add an assert sy-dbcnt <= 1 immediately after the selection instead of getting fancy and raising an exception.
Asserts are a simple and effective way of making people aware that there is something (seriously) wrong.

Even if your code is flawless and wouldn't insert another entry someone else may break that assumption.
Zero trust!

@ErikPross
Copy link

Like BiberM states above, when you are not using the primary key for selection the possibility exists that more than one row meets your WHERE clause. You may then want to select only the newest line for example.

With SELECT SINGLE the option ORDER BY date DESCENDING, so you only get the newest line is not allowed. In old systems the syntax check doesn't complain, but the ordering is carried out after the single selection, so it does nothing. In newer systems it won't let you add the ORDER BY.

When using SELECT UP TO 1 ROWS / ENDSELECT you can use the ORDER BY and get only the newest record that you wanted. I do agree that using an empty SELECT / ENDSELECT block in your code looks stupid. Maybe SAP should improve the SELECT SINGLE or make a variation.

So the only time I would ever use SELECT / ENDSELECT these days is in combination with UP TO 1 ROWS and knowing that my key will return more than one row AND I actually care which one of those I get back.

@Jelena-P
Copy link

It's funny that this discussion still continues 3 years later. :) I came back here because I just saw someone write (in a new program!) SELECT... ENDSELECT and they did have primary key. Maybe it's time to reach some compromise and finally add a suggestion to Clean ABAP?

  1. SELECT SINGLE can be used safely when specifying full key. E.g. SELECT ... FROM mara WHERE matnr = p_matnr. There is no way you get more than one record.

  2. If you do not have full key, use SELECT UP TO 1 ... ORDER BY... as a general all-purpose guideline.

@kjetil-kilhavn has an interesting example but I wouldn't muddy the waters with it. I'm sure everyone will keep their own preferences. Personally, 'll keep using SELECT SINGLE in cases where I really don't care if there could be more than one record, hehe. :) But this is one of the things where you first learn how to do it properly, understand the consequences, then you could try taking liberties.

@kjetil-kilhavn
Copy link
Contributor

kjetil-kilhavn commented May 16, 2024

@kjetil-kilhavn has an interesting example but I wouldn't muddy the waters with it. I'm sure everyone will keep their own preferences. Personally, 'll keep using SELECT SINGLE in cases where I really don't care if there could be more than one record, hehe. :) But this is one of the things where you first learn how to do it properly, understand the consequences, then you could try taking liberties.

I have seen time-dependent HR master data with exactly the case that I described and I think it actually occurred after my comment a year ago. In our case, the incorrect data occurred after ALE transfers and were corrected by subsequent ALE transfers. No trace, at least where I would normally look for one. I think we would have to spend an insane amount of time, or be very lucky, to figure out that the root cause of an incorrect tax calculation was that the employee at that particular day had two valid records in an HR master data table where that isn't supposed to be possible.

So as I said, even if your code is flawless ... but of course it depends on what the consequences are.
There are cases where it isn't critical if the wrong record is selected, but there are also a lot of examples where I definitely don't want processing to continue in such cases. Salary calculations, tax calculation and reporting - or any reporting to the government for that matter, are a few such examples. To make matters worse incorrect data can in rare cases be corrected without any obvious trace, making it nearly impossible to find out what caused the incorrect result.

The reason we found the incorrect HR master data was that one of business experts told me that a specific functionality in purchasing didn't work for him. That was weird, and definitely wrong, and after some debugging the root cause was, much to my surprise, found to be this problem with his HR master data. So, this was not a critical case and the problem occurred in SAP standard code which assumed there would be only one valid record (SAP standard info type as well in other words). Around one per thousand employees had incorrect data in the info type.

Someone getting mad at me because a report crashes is a small price to pay to avoid one day (hah!) or ten (more likely) of debugging. We all know who gets the blame and the burden of proof when they eventually figure out that the report produced an incorrect result. It is not unlikely that the data are correct 6 months later when they discover that the result from the report was incorrect, so good luck with that root cause analysis.

UPDATE:
Looking back at my comment I see that I could have been clearer about the distinction between cases where it is critical that the assumption holds, and cases where it isn't critical.

I agree with the suggestion from @Jelena-P that Clean ABAP should recommend SELECT SINGLE when you have the full key specified and SELECT ... ENDSELECT as the recommendation all other cases.
Then there is at least a recommendation, and people who want to ignore are as always free to do so ...

Horst Keller writes in his blog from 2015 that "SELECT SINGLE should not be used with partial key specifications." so I don't understand why we even have this debate :-D

@kjetil-kilhavn
Copy link
Contributor

kjetil-kilhavn commented May 16, 2024

SELECT SINGLE *
  FROM bkpf
  WHERE belnr = document_number.
IF sy-subrc = 0.
  "Do something ... like pay document amount to bank_account
ENDIF.

What could possibly go wrong? There is no need to specify the full key or check that there aren't actually two documents with that number, because everyone knows we defined separate document number ranges per company code and year when we set up the system 27 years ago. (Well, at least me and the other 2 people who have been here those 27 years.) If someone decides to change such a central thing as the document number range setup then surely they will ring the bell and tell everyone that every solution has to be re-tested to ensure that this change won't cause any problems.

Oh, the lessons learned over the years. I've seen an ABAP developer define a table for funds centre data without FIKRS as part of the table key. Why? Because this customer had only one FM Area. I was nearly speechless.

Not to mention my own mistakes that I have learnt from, but let's not get personal and nasty here :-)

@pokrakam
Copy link
Contributor

pokrakam commented May 16, 2024

I hear you @kjetil-kilhavn and share your pain 🙂

But... I do believe there are many scenarios where we truly don't care what record is returned (e.g. existence check). There I think it's perfectly acceptable to use an ambiguous SELECT SINGLE for simplicity. But I believe ATC should be in use and warn about these as it forces the dev to at least think about it. I am OK with:

METHOD is_warehouse_order. 
  SELECT SINGLE @abap_true FROM ekpo
    WHERE ebeln = po_nr 
      AND werks = c_warehouse
    INTO @DATA(result).            "#EC CI_NOORDER
ENDMETHOD.

Or maybe one could make it an exemptable check to force a second pair of eyes onto it.

@kjetil-kilhavn
Copy link
Contributor

kjetil-kilhavn commented May 19, 2024

I don't disagree @pokrakam – there are many scenarios where we just want to check for existence. My point is that there are also cases were e.g. the existence of more than one record indicate that an important assumption is false.

I like your example, though in my opinion that example doesn't become complicated if you change it to use select ... endselect. I just don't see any great simplification gained by using select single. Personally I therefore value the "documentation" that is implicit in the use of select ... endselect more than the two lines of code saved by using select single.

But as @Jelena-P pointed out, most of us who already have a preference will likely stick to it regardless of what Clean ABAP says. There is still value in recommending something in Clean ABAP for new developers.

@pokrakam
Copy link
Contributor

@kjetil-kilhavn I think we broadly agree, and perhaps some of it is just a matter of opinion.
Personally I dislike most empty loop...endloop and select...endselect constructs, as I usually double back and read it again. It looks like something incomplete - is something missing, is it a content check or did the developer start and forget to come back to this piece of code?

So therefore my preference is to avoid it unless necessary - if, as per your example, it DOES matter if the assumption turns out to be false then I fully agree we should use a validated construct. In this case I personally prefer a table selection for absolute clarity:

SELECT ... INTO TABLE @DATA(orders). 
ASSERT sy-dbcnt < 2.  "or
CASE sy-dbcnt. 
  WHEN 0.
    do_when_empty( ).
  WHEN 1.
    do_stuff_with( orders[ 1 ] ).
  WHEN OTHERS. 
    RAISE EXCEPTION NEW cx_bapi_error( ).
ENDCASE.

If it really is a strong assumption then that may even be case for unchecked exceptions cx_dynamic_check / cx_no_check.

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

No branches or pull requests