December 13, 2016 at 11:54 am
patrickmcginnis59 10839 (12/13/2016)
TheSQLGuru (12/13/2016)
A) Regarding index reorg, I don't think you get "random crap in your database" because of it while running NOLOCK SELECTs. You may get committed bad data OUT on said SELECTs though.If you prepare an update based on erroneous data from nolock-ed selects then the entire routine is suspect in my opinion. I do realize opinions differ on this, I'm sure theres a school of thought that says the update is fine, but the select is bad.
-- A COUPLE OF TABLES
CREATE TABLE TEST1 ( KEY1 INT, DATA1 INT)
CREATE TABLE TEST2 ( KEY1 INT, DATA1 INT)
-- IN FIRST WINDOW START A TRANSACTION
BEGIN TRAN
INSERT INTO TEST1 SELECT 1,1
-- IN SECOND WINDOW ACT UPON THE UNCOMMITTED TRANSACTION WITH A NOLOCK
INSERT INTO TEST2 (KEY1, DATA1) SELECT KEY1, DATA1 FROM TEST1 WITH (NOLOCK)
-- IN FIRST WINDOW ROLL BACK THE TRANSACTION
ROLLBACK
-- VIEW RESULTS
SELECT * FROM TEST1
SELECT * FROM TEST2
I'm sure that folks don't include the source select in the transaction as part of the code that I on the other hand want to execute properly. That doesn't make me like (NOLOCK) any better.
Even more so if you're compiling data in separate steps to prepare for an update, and your compiling steps have (NOLOCK)
Whether doing update routines in multiple steps is a good idea or not is another question, I think its possible to do them correctly but even here I see racy code from experienced posters, in any case using nolock to gather data for updates is not my favorite programming paradigm.
Ahh, I see your scenario now. Pull data out and do something based on that. I see less of that (albeit FAR more than I should) at clients than I see the NOLOCKs used on "report" or read-only-no-action SELECTs.
I also note that your scenario doesn't actually touch on the issue that can happen with REORG and NOLOCK, which I thought you were speaking to. Those REORG page swaps don't get rolled-back - they get committed. Yet the page movement from REORGs (and other similar-effect actions) can in fact result in your getting COMMITTED BAD DATA, which to me is a vastly different thing than most people "accept" with SELECT WITH NOLOCKs (even if they REALLY don't understand the ramifications) - namely seeing UNCOMMITTED data that could change after their SELECT is complete.
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
December 13, 2016 at 12:19 pm
TheSQLGuru (12/13/2016)
patrickmcginnis59 10839 (12/13/2016)
TheSQLGuru (12/13/2016)
A) Regarding index reorg, I don't think you get "random crap in your database" because of it while running NOLOCK SELECTs. You may get committed bad data OUT on said SELECTs though.If you prepare an update based on erroneous data from nolock-ed selects then the entire routine is suspect in my opinion. I do realize opinions differ on this, I'm sure theres a school of thought that says the update is fine, but the select is bad.
-- A COUPLE OF TABLES
CREATE TABLE TEST1 ( KEY1 INT, DATA1 INT)
CREATE TABLE TEST2 ( KEY1 INT, DATA1 INT)
-- IN FIRST WINDOW START A TRANSACTION
BEGIN TRAN
INSERT INTO TEST1 SELECT 1,1
-- IN SECOND WINDOW ACT UPON THE UNCOMMITTED TRANSACTION WITH A NOLOCK
INSERT INTO TEST2 (KEY1, DATA1) SELECT KEY1, DATA1 FROM TEST1 WITH (NOLOCK)
-- IN FIRST WINDOW ROLL BACK THE TRANSACTION
ROLLBACK
-- VIEW RESULTS
SELECT * FROM TEST1
SELECT * FROM TEST2
I'm sure that folks don't include the source select in the transaction as part of the code that I on the other hand want to execute properly. That doesn't make me like (NOLOCK) any better.
Even more so if you're compiling data in separate steps to prepare for an update, and your compiling steps have (NOLOCK)
Whether doing update routines in multiple steps is a good idea or not is another question, I think its possible to do them correctly but even here I see racy code from experienced posters, in any case using nolock to gather data for updates is not my favorite programming paradigm.
Ahh, I see your scenario now. Pull data out and do something based on that. I see less of that (albeit FAR more than I should) at clients than I see the NOLOCKs used on "report" or read-only-no-action SELECTs.
I also note that your scenario doesn't actually touch on the issue that can happen with REORG and NOLOCK, which I thought you were speaking to. Those REORG page swaps don't get rolled-back - they get committed. Yet the page movement from REORGs (and other similar-effect actions) can in fact result in your getting COMMITTED BAD DATA, which to me is a vastly different thing than most people "accept" with SELECT WITH NOLOCKs (even if they REALLY don't understand the ramifications) - namely seeing UNCOMMITTED data that could change after their SELECT is complete.
I really think its as you describe it, the pages are in motion and are probably read at a time they shouldn't be, but I do remember having more bad updates when we did the reorgs during weekdays / busydays.
The bad code I'm complaining about was also with the selects either part of an update or preparing for one, they really didn't have to rollback as in my contrived example, heck I wish it were that easy. NOLOCK crap in reality seems really random because it seems pretty much non deterministic in nature.
December 13, 2016 at 1:51 pm
hijacked thread 🙂
December 14, 2016 at 6:33 am
Indianrock (12/13/2016)
hijacked thread 🙂
we are altering the thread. pray we do not alter it any further.
December 14, 2016 at 8:17 am
patrickmcginnis59 10839 (12/14/2016)
Indianrock (12/13/2016)
hijacked thread 🙂we are altering the thread. pray we do not alter it any further.
HAH!! :hehe:
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
December 14, 2016 at 1:41 pm
First minor change to the code you posted earlier:
SELECT DISTINCT TOP 2000
CollateralGroupRequest02.SERVICE_REQUEST_ID AS PrimaryKey ,
Client18.SHORT_NAME AS ClientShortName ,
EeeReason19.DESCRIPTION AS EeeReason ,
AccountProperty110.MODIFIED_MANUFACTURER_ID AS VIN ,
Account15.CATEGORY AS AccountType ,
FollowupEntity111.ENTITY_CODE AS DealerID ,
FollowupEntity111.NORMALIZED_ENTITY_CODE AS FollowupEntity111_NORMALIZED_ENTITY_CODE58 ,
Account15.FINANCED_DATE AS FinancedDate ,
Account15.BOOKED_DATE AS BookedDate ,
AccountProperty110.EXPECTED_TITLING_STATE_ABBR AS ExpectedTitlingState ,
BusinessUnit112.LONG_NAME AS BusinessUnit ,
BusinessUnit112.SHORT_NAME AS BusinessUnitId ,
CollateralGroupRequest02.SERVICE_REQUEST_STATUS AS Status ,
AccountOwnershipDocSummary16.STATUS AS AccountStatus ,
AccountOwnershipDocSummary16.BORROWER_FULL_NAMES AS BorrowerFullNames ,
Account15.CUSTOM_ATTRIBUTE_1 AS AccountNumber ,
Account15.CUSTOM_ATTRIBUTE_2 AS LoanNumber ,
Account15.CUSTOM_ATTRIBUTE_3 AS LoanSuffix ,
Account15.CUSTOM_ATTRIBUTE_4 AS Branch ,
CollateralGroupRequest02.EEE_DATE AS EEEDate ,
CollateralGroupRequest02.EEE_CAUSE AS CauseOfEEE ,
CollateralGroupRequest02.REQUEST_TRANSACTION_TYPE AS RequestType ,
Jurisdiction113.SHORT_NAME AS TMState ,
Account15.RECOVERY_STATUS AS RecoveryCode ,
Account15.USER_DEFINED_1 AS UserDef1 ,
Account15.USER_DEFINED_2 AS UserDef2 ,
Account15.USER_DEFINED_3 AS UserDef3 ,
LienholderStatusCode114.STATUS_CODE AS LienholderStatus ,
AccountProperty110.VEHICLE_TYPE AS CollateralType ,
Account15.SUB_CATEGORY AS AccountSubType ,
-- Change here
oa1.ReminderDate AS ReminderDate ,
oa1.ReminderUser AS ReminderUser
--
FROM SERVICE_REQUEST AS CollateralGroupRequest02
INNER JOIN ( SERVICED_COLLATERAL_GROUP_ITEM AS ServicedCollateralGroupItem13
INNER JOIN ( SERVICED_COLLATERAL_GROUP_ITEM AS ServicedAccount14
INNER JOIN ( ACCOUNT AS Account15
INNER JOIN ACCOUNT_OWNERSHIP_DOC_SUMMARY AS AccountOwnershipDocSummary16 ON Account15.ACCOUNT_ID = AccountOwnershipDocSummary16.ACCOUNT_ID
INNER JOIN PROPERTY AS AccountProperty110 ON Account15.ACCOUNT_ID = AccountProperty110.ACCOUNT_ID
LEFT OUTER JOIN LEGAL_ENTITY AS FollowupEntity111 ON Account15.FOLLOWUP_ENTITYLEGAL_ENTITY_ID = FollowupEntity111.LEGAL_ENTITY_ID
LEFT OUTER JOIN BUSINESS_UNIT AS BusinessUnit112 ON Account15.BUSINESS_UNIT_ID = BusinessUnit112.BUSINESS_UNIT_ID
LEFT OUTER JOIN LIENHOLDER_STATUS_CODE AS LienholderStatusCode114 ON Account15.LIENHOLDER_STATUS_CODE_ID = LienholderStatusCode114.LIENHOLDER_STATUS_CODE_ID
) ON ServicedAccount14.ACCOUNT_ID = Account15.ACCOUNT_ID
) ON ServicedCollateralGroupItem13.SERVICED_COLLATERAL_GROUP_ITEM_ID = ServicedAccount14.SERVICED_COLLATERAL_GROUP_ITEM_ID
) ON CollateralGroupRequest02.SERVICE_REQUEST_ID = ServicedCollateralGroupItem13.COLLATERAL_GROUP_REQUESTSERVICE_REQUEST_ID
INNER JOIN ORGANIZATION AS Client18 ON CollateralGroupRequest02.CLIENT_ID = Client18.ORGANIZATION_ID
LEFT OUTER JOIN EEE_REASON AS EeeReason19 ON CollateralGroupRequest02.EEE_REASON_ID = EeeReason19.EEE_REASON_ID
INNER JOIN ORGANIZATION AS Jurisdiction113 ON CollateralGroupRequest02.JURISDICTION_ID = Jurisdiction113.ORGANIZATION_ID
-- Change here
OUTER APPLY ( SELECT TOP 1
ExternalUser13.USERNAME AS ExternalUser13_USERNAME13 ,
ReminderWorkItem02.REMIND_DATE AS ReminderWorkItem02_REMIND_DATE1
FROM WORK_QUEUE_ITEM AS ReminderWorkItem02
INNER JOIN USR AS ExternalUser13 ON ReminderWorkItem02.ASSIGNED_USER_ID = ExternalUser13.USR_ID
WHERE ( ( ReminderWorkItem02.ACCOUNT_ID = Account15.ACCOUNT_ID
AND ReminderWorkItem02.BUSINESS_PROCESS_STATUS = 'Open'
AND ReminderWorkItem02.NAME = 'REMINDER'
AND ReminderWorkItem02.SECURED_ORGANIZATIONORGANIZATION_ID = Account15.CLIENT_ID
)
AND ( ( ReminderWorkItem02.CONCRETE_TYPE IN (
'Fdi.Workflow.Po.ReminderWorkItem' )) )
)
ORDER BY ReminderWorkItem02.REMIND_DATE -- 2
) oa1(ReminderUser,ReminderDate)
--
WHERE ( (CollateralGroupRequest02.CLIENT_ID = 11330
AND Account15.CLIENT_ID = 11330
AND CollateralGroupRequest02.EEE_DATE IS NOT NULL
AND ( ( CollateralGroupRequest02.CONCRETE_TYPE = 'Fdi.Po.FollowUpRequest'
AND CollateralGroupRequest02.BUSINESS_PROCESS_STATUS IN (
'Open', 'Closed' )
AND AccountOwnershipDocSummary16.STATUS = 'NO_TITLE_PM'
)
OR ( CollateralGroupRequest02.CONCRETE_TYPE = 'Fdi.Po.TitleMaintenanceRequest'
AND CollateralGroupRequest02.BUSINESS_PROCESS_STATUS = 'Open'
AND AccountOwnershipDocSummary16.STATUS = 'TITLE_MAINTENANCE_REQUEST_SENT'
)
OR ( CollateralGroupRequest02.CONCRETE_TYPE = 'Fdi.Po.DuplicateTitleRequest'
AND CollateralGroupRequest02.BUSINESS_PROCESS_STATUS = 'Closed'
)
OR ( CollateralGroupRequest02.CONCRETE_TYPE = 'Fdi.Po.DirectLendingServiceRequest'
AND CollateralGroupRequest02.BUSINESS_PROCESS_STATUS = 'Open'
AND AccountOwnershipDocSummary16.STATUS = 'NO_TITLE_PM'
OR ( CollateralGroupRequest02.SERVICE_REQUEST_STATUS = 'DocumentsReturned'
AND CollateralGroupRequest02.BUSINESS_PROCESS_STATUS = 'Closed'
AND AccountOwnershipDocSummary16.STATUS = 'NO_TITLE_PM'
)
)
OR ( CollateralGroupRequest02.CONCRETE_TYPE = 'Fdi.Po.AdHocRequest'
AND CollateralGroupRequest02.BUSINESS_PROCESS_STATUS = 'Open'
)
)
AND ( Account15.CUSTOM_ATTRIBUTE_4 = '01102' ))
)
ORDER BY CollateralGroupRequest02.EEE_DATE -- 20
There could be additional changes, like removing the DISTINCT from if it turns out it isn't needed (would need to test that to see), rework the joins perhaps (again, testing would be definitely needed).
December 14, 2016 at 3:29 pm
Thanks Lynn I'll give it a run
December 14, 2016 at 4:16 pm
You may have missed it Lynn, but this is an ORM app. I doubt very much it is going to be reworking that query much at all, almost certainly not to add in an OUTER APPLY to that TOP 1 query. 🙁
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
December 14, 2016 at 6:03 pm
TheSQLGuru (12/14/2016)
You may have missed it Lynn, but this is an ORM app. I doubt very much it is going to be reworking that query much at all, almost certainly not to add in an OUTER APPLY to that TOP 1 query. 🙁
I know it is. He can test what I wrote and compare it to what the ORM generated and hopefully see a difference. May help show the powers that be that ORMs don't always know what's best. Not counting on it.
I have a similar problem where I work except it is developer based code embedded (crafted dynamically in many cases). A PR was entered regarding on piece of code and the developer reworking it pulled all out of the app, put it in a stored procedure, and was surprised that it performed better but asked me to look at it and explain why it worked better. It was nice finally getting that opportunity and I did. Also told him what he wrote was a good start but that it still had many of the flaws of the original code in the app. I took the time to rewrite that code and provide that, although I have heard nothing back as of yet and that was a several weeks ago.
If you can show that ORM code (or developer code for that matter) can be improved, it is always a good a thing. At some point, things may change.
Oh, that developer asked what they could do to improve other aspects of the web app code. My answer, rip out all the embedded SQL, go to calling stored procedures, and put the code into the database, just be sure that it is written to be highly performant (I know, not a real word) and scalable.
December 14, 2016 at 7:53 pm
What a weird code!
.... SERVICED_COLLATERAL_GROUP_ITEM AS ServicedAccount14
INNER JOIN ( ACCOUNT AS Account15
INNER JOIN ACCOUNT_OWNERSHIP_DOC_SUMMARY AS AccountOwnershipDocSummary16 ON Account15.ACCOUNT_ID = AccountOwnershipDocSummary16.ACCOUNT_ID
INNER JOIN PROPERTY AS AccountProperty110 ON Account15.ACCOUNT_ID = AccountProperty110.ACCOUNT_ID
LEFT OUTER JOIN LEGAL_ENTITY AS FollowupEntity111 ON Account15.FOLLOWUP_ENTITYLEGAL_ENTITY_ID = FollowupEntity111.LEGAL_ENTITY_ID
LEFT OUTER JOIN BUSINESS_UNIT AS BusinessUnit112 ON Account15.BUSINESS_UNIT_ID = BusinessUnit112.BUSINESS_UNIT_ID
LEFT OUTER JOIN LIENHOLDER_STATUS_CODE AS LienholderStatusCode114 ON Account15.LIENHOLDER_STATUS_CODE_ID = LienholderStatusCode114.LIENHOLDER_STATUS_CODE_ID
) ON ServicedAccount14.ACCOUNT_ID = Account15.ACCOUNT_ID
All those brackets need to be definitely removed.
They do not do anything but confusing everybody and slowing down the search for a better plan.
And look at this:
INNER JOIN ( SERVICED_COLLATERAL_GROUP_ITEM AS ServicedCollateralGroupItem13
INNER JOIN ( SERVICED_COLLATERAL_GROUP_ITEM AS ServicedAccount14
INNER JOIN ( ACCOUNT AS Account15
....
) ON ServicedCollateralGroupItem13.SERVICED_COLLATERAL_GROUP_ITEM_ID = ServicedAccount14.SERVICED_COLLATERAL_GROUP_ITEM_ID
The same table is joined to itself by the same column.
Apart of absolute stupidity of such a join (ServicedCollateralGroupItem13 is not used anywhere but within this join),
what else do you need to bring the server down?
And the cherry on the pie - all those "OR"s - just to make any kind of indexes or statistics totally unusable.
_____________
Code for TallyGenerator
December 14, 2016 at 7:55 pm
We can blame the ORM tool for everything (probably even deserving), but in this case, I reckon, the query it's generated only reflects the mess in the code upstairs.
_____________
Code for TallyGenerator
December 14, 2016 at 8:04 pm
Yes ORM-generated sql. Ours is a version called Domain Objects written by a former developer here. Others include Entity Framework and Hibernate. That sql code was not written by a human being.
So it allows a company to throw up a web product quickly where developers don't have to know any sql.
Might be fine if you start turning away customers once your largest table has 1 million rows.
December 15, 2016 at 6:15 am
Lynn Pettis (12/14/2016)
TheSQLGuru (12/14/2016)
You may have missed it Lynn, but this is an ORM app. I doubt very much it is going to be reworking that query much at all, almost certainly not to add in an OUTER APPLY to that TOP 1 query. 🙁I know it is. He can test what I wrote and compare it to what the ORM generated and hopefully see a difference. May help show the powers that be that ORMs don't always know what's best. Not counting on it.
I have a similar problem where I work except it is developer based code embedded (crafted dynamically in many cases). A PR was entered regarding on piece of code and the developer reworking it pulled all out of the app, put it in a stored procedure, and was surprised that it performed better but asked me to look at it and explain why it worked better. It was nice finally getting that opportunity and I did. Also told him what he wrote was a good start but that it still had many of the flaws of the original code in the app. I took the time to rewrite that code and provide that, although I have heard nothing back as of yet and that was a several weeks ago.
If you can show that ORM code (or developer code for that matter) can be improved, it is always a good a thing. At some point, things may change.
Oh, that developer asked what they could do to improve other aspects of the web app code. My answer, rip out all the embedded SQL, go to calling stored procedures, and put the code into the database, just be sure that it is written to be highly performant (I know, not a real word) and scalable.
You are preaching to the choir my friend! 🙂 Earlier this year I was brought in to a new client that had paid a very large sum of money to a development firm to build a new app for them. Pure EF. After maybe 10ish days of consulting over a couple of months they decided essentially replace the entire app it was so bad. Fortunately I got the most egregious things fixed and taught them how to find and fix others so they could at least make the app functional at the scale it needed to be for real-world use. Just so sad how bad most developers (and development companies/ISVs) are when it comes to building efficient, scalable code these days (regardless of data platform)!! :crazy:
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
December 15, 2016 at 6:29 am
Indianrock (12/14/2016)
Yes ORM-generated sql. Ours is a version called Domain Objects written by a former developer here. Others include Entity Framework and Hibernate. That sql code was not written by a human being.So it allows a company to throw up a web product quickly where developers don't have to know any sql.
Might be fine if you start turning away customers once your largest table has 1 million rows.
From the article:
"First, some best practices:
• Do not use built-in connection pooling. It is not intended for production use.
• Do not use default caching. It too is not intended for production use.
• For complicated queries, use inline SQL or stored procedures. Hibernate is not made to cover every scenario."
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
December 15, 2016 at 6:51 am
Indianrock (12/14/2016)
Yes ORM-generated sql. Ours is a version called Domain Objects written by a former developer here. Others include Entity Framework and Hibernate. That sql code was not written by a human being.
I'm not as familiar with ORM type stuff as I should be, but if Domain Objects is not a mainstream package, that compounds your issues right there. At least with EF or Hibernate you can commiserate with like minded users on how to hit up some of the downsides of ORM, but with some random ORM written by some random developer dreaming of the next big thing, you inherit all the work of making an ORM work without the benefits of an active developer community.
Might want to run that situation by management, it might not actually be an ORM problem, it might be more of an "ex employee left a mess." Not trying to speak for or against ORM's, just talking about the situation you described.
Viewing 15 posts - 31 through 45 (of 56 total)
You must be logged in to reply to this topic. Login to reply