GoFish icon indicating copy to clipboard operation
GoFish copied to clipboard

SQL: ORDER BY clause is invalid when sorting on a memo column...

Open mattslay opened this issue 6 years ago • 7 comments

On Sun, May 13, 2018, 8:20 AM KoenP, wrote:

... I now get error, if the grid is sorted by a certain column:

Message: SQL: ORDER BY clause is invalid. Command: Select * From (Select * From (lcSearchEngineResultsAlias) Order By &lcOrderBy) source Into Cursor gf_temp_csr ReadWrite Method: GOFISH.FILLSEARCHRESULTSCURSOR

Needles to say that I have meanwhile redownloaded and reinstalled GoFish.app today however same effect.

Could this be related to other feautures which we face since the last windows update ( Beaufify works differently for example) ?

Any idea how to cure this?

Regards,

Koen

========================== Mike Yearwood offere a fix by email:

Hey all

This error happens because order by cannot order by a memo field, proccode in this case.

I made a little fix. Hopefully others find this helpful.

Mike

============================ 2018-06-20 23:04 GMT+02:00 Mike Yearwood

Hey there

I found a mistake in my code. Here's the update.

*Make a cursor with a proccode that can be ordered.

*!*         Select * From (Select * From (lcSearchEngineResultsAlias) Order By &lcOrderBy) source ;

*!*         Into Cursor gf_temp_csr ReadWrite

lnFields = AFIELDS(laFields,m.lcSearchEngineResultsAlias)

lcFields=laFields[1,1]

FOR lnI = 2 TO lnFields

      m.lcFields = m.lcFields + ","+ALLTRIM(laFields[m.lnI,1])

ENDFOR

m.lcFields =STRTRAN(m.lcFields,",PROCCODE","")

m.lcFields = STRTRAN(m.lcFields,",LONGPROCCODE","")

m.lcFields1 = m.lcFields+",left(proccode,254) as proccode,proccode as longproccode"

m.lcFields2 = m.lcFields+",longproccode as proccode"

 

SELECT &lcFields1 FROM(m.lcSearchEngineResultsAlias) INTO CURSOR (m.lcSearchEngineResultsAlias)readwrite

 

SELECT &lcFields2 From (Select * From(m.lcSearchEngineResultsAlias) Order By &lcOrderBy) source ;

      Into Cursor gf_temp_csrReadWrite

mattslay avatar Jun 21 '18 08:06 mattslay

just tried out the newest release of "GoFish5.APP" V 5.0.170 and it seems the suggested fix has not been applied, because when i (at first accidently) click to order by a supposed memo-field ("proccode", "statement" or "code") then Gofish throws an error "SQL: ORDERBY clause is invalid." !

After clicking "Suspend" this is what the Debugger shows (hope this helps): 2021-06-10_16h28_10

PatrickvonDeetzen avatar Jun 10 '21 14:06 PatrickvonDeetzen

Issue still marked as open, so when will this error be fixed in a new "GoFish5.app" release, since there already seems to be a solution available?

@PatrickvonDeetzen - I will look into this and release a new build once I can go thorough testing with the submitted code fix I received from Mike Yearwood.

mattslay avatar Jun 10 '21 14:06 mattslay

Wow, great news! 👍
Thanks in advance. :)

PatrickvonDeetzen avatar Jun 10 '21 14:06 PatrickvonDeetzen

I did not realize this gets called when you click on any column header. Funny that no one pointed that out. Here is a revamped FillSearchResultsCursor

Local lcCollate, lcField, lcOrderBy, lcSearchEngineResultsAlias, lcSearchResultsAlias,;
lcSecondarySortField, lcSecondarySortOrder, lcSortField, lcSortOrder, lcTertiarySortField,;
lcTertiarySortOrder, lnSelect, loColumn, lcWord

lcSearchEngineResultsAlias = Thisform.oSearchEngine.cSearchResultsAlias
lcSearchResultsAlias = Thisform.cSearchResultsAlias

lnSelect = Select()
lcOrderBy = ''

*** JRN 11/06/2015 : using !Empty(Field( to provide for case of changed field names in GF 5
*-- Construct Order By clause for Select statement from stored form property
If !Empty(Thisform.cSortField) and !Empty(Field(Thisform.cSortField, m.lcSearchEngineResultsAlias))
	lcSortField = Thisform.cSortField
	lcSortOrder = Thisform.cSortOrder
	If (Empty(m.lcSortOrder) or ('ASC' $ Upper(m.lcSortOrder)))
		lcSortOrder = 'ASC'
	Endif
	lcOrderBy = m.lcSortField + ' ' + m.lcSortOrder 
Endif

If !Empty(Thisform.cSortFieldSecondary) and !Empty(Field(Thisform.cSortFieldSecondary, m.lcSearchEngineResultsAlias))
	lcSecondarySortField = Thisform.cSortFieldSecondary
	lcSecondarySortOrder = Thisform.cSortOrderSecondary
	If (Empty(m.lcSecondarySortOrder) or ('ASC' $ Upper(m.lcSecondarySortOrder)))
		lcSecondarySortOrder = 'ASC'
	EndIf
	lcOrderBy = m.lcOrderBy + ', ' + m.lcSecondarySortField + ' ' + m.lcSecondarySortOrder
Endif

If !Empty(Thisform.cSortFieldTertiary) and !Empty(Field(Thisform.cSortFieldTertiary, m.lcSearchEngineResultsAlias))
	lcTertiarySortField = Thisform.cSortFieldTertiary
	lcTertiarySortOrder = Thisform.cSortOrderTertiary
	If (Empty(m.lcTertiarySortOrder) or ('ASC' $ Upper(m.lcTertiarySortOrder)))
		lcTertiarySortOrder = 'ASC'
	EndIf
	lcOrderBy = m.lcOrderBy + ', ' + m.lcTertiarySortField + ' ' + m.lcTertiarySortOrder
Endif

If Empty(m.lcOrderBy)
	lcOrderBy = 'filename, class, name, methodname'
	thisform.cSortField = GetWordNum(m.lcOrderBy, 1, ',')
Else
	lcOrderBy = Alltrim(m.lcOrderBy, 1, ',')
EndIf

Thisform.cOrderBy = m.lcOrderBy

*-- Make header bold for primary sort column
If Thisform.lInit
	*Mike Yearwood - Reset all column headers at once.
	Thisform.oGrid.SetAll("FontBold",.F.,"Header")
	lcWord=THISFORM.cSortField
	For each loColumn in Thisform.oGrid.Columns
		*loColumn.header1.FontBold = .f.
		*lcField = GetWordNum(loColumn.ControlSource, 2, '.')
		*If Upper(GetWordNum(m.loColumn.ControlSource, 2, '.')) = Upper(GetWordNum(m.lcOrderBy, 1, ', '))
		If Upper(GetWordNum(m.loColumn.ControlSource, 2, '.')) = m.lcWord
			loColumn.header1.FontBold = .t.
			*Can there be 2 primary sort columns?
			EXIT
		Endif
	Endfor
EndIf

*-- This gives us case insensitive sorting...
lcCollate = Set('Collate')
Try 
	Set Collate To 'General'
Catch to loException
	
EndTry

lcOrderBy = m.lcOrderBy + ', filename, class, name, methodname, MatchStart'
*-- Create the local cursor if it does not already exist -----------------------

*Cannot order by memo fields, so build a field list with any existing memos
*in the lcOrderBy added as "left(memofieldname,250) as LONGmemofieldname"
*Then use these LONGmemofieldnames in the lcMyOrderBy
LOCAL lnField, lnFields, lcFields, lcLongFields, lcLongClauses, lcMyOrderBy

*Make a fully tilde-delimted lcOrderBy for scanning matching memofield names.
*Convert spaces to ~ and , to "~,"
lcMyOrderBy="~"+chrtran(m.lcOrderBy," ","~")
lcMyOrderBy=strtran(m.lcMyOrderBy,",","~,")
if right(m.lcMyOrderBy,1)<>'~'
	lcMyOrderBy=m.lcMyOrderBy+'~'
endif

lnFields=AFields(laFields,m.lcSearchEngineResultsAlias)

*Build a set of 'original' field names
lcOriginalFields=laFields[1,1]
for lnField=2 to m.lnFields
	lcOriginalFields=m.lcOriginalFields+","+laFields[m.lnField,1]
endfor lnField

*Build a conditional set of LONGFIELDS
*Stupid afields has to be scanned with a primitive loop
*to find memos to then see if they are in lcMyOrderBy,
*instead of the more elegant possibilities from a cursor.
store '' to lcLongFields, lcLongClauses
FOR lnField = 1 to m.lnFields
	IF laFields[m.lnField,2]<>"M"
		LOOP
	ENDIF
	IF "~"+laFields[m.lnField,1]+"~"$m.lcMyOrderBy
		lcLongField="LONG"+laFields[m.lnField,1]
		lcLongClauses=m.lcLongClauses+",left("+laFields[m.lnField,1]+",250) as "+m.lcLongField
		lcLongFields=m.lcLongFields+","+m.lcLongField
		*replace the original memo field name, such as statement with longstatement
		lcMyOrderBy=strtran(m.lcMyOrderBy,"~"+laFields[m.lnField,1]+"~","~"+m.lcLongField+"~")
	ENDIF
ENDFOR lnField
if right(m.lcLongClauses,1)=","
	lcLongClauses=left(m.lcLongClauses,len(m.lcLongClauses)-1)
endif
if right(m.lcLongFields,1)=","
	lcLongFields=left(m.lcLongFields,len(m.lcLongFields)-1)
endif
lcMyOrderBy=strtran(m.lcMyOrderBy,"~,",",")
if left(m.lcMyOrderBy,1)="~"
	lcMyOrderBy=substr(m.lcMyOrderBy,2)
endif
if right(m.lcMyOrderBy,1)="~"
	lcMyOrderBy=left(m.lcMyOrderBy,len(m.lcMyOrderBy)-1)
endif
lcMyOrderBy=chrtran(m.lcMyOrderBy,"~"," ")

If !Used(m.lcSearchResultsAlias)
*!*		Select * From (Select * From (lcSearchEngineResultsAlias) Order By &lcOrderBy) source;
*!*		 Into Cursor (lcSearchResultsAlias) ReadWrite 

	Select &lcOriginalFields. From (Select * &lcLongClauses. From (m.lcSearchEngineResultsAlias) Order By &lcMyOrderBy.) source;
	 Into Cursor (m.lcSearchResultsAlias) ReadWrite 

Else && Otherwise, Zap it and rebuild from GFSE cursor...
	
	Thisform.ClearSearchResultsCursor() && Zap it
	*-- Build a temp local cursor in correct initial order
	
	*TODO: The orderby clause here could fail since some field names were changed
	*      in relase 4.3.015. Need to handle this...
	 
*!*		Select * From (Select * From (lcSearchEngineResultsAlias) Order By &lcOrderBy) source ;
*!*		 Into Cursor gf_temp_csr ReadWrite

*!*		Select * From (Select * &lcLongClauses. From (m.lcSearchEngineResultsAlias) Order By &lcMyOrderBy.) source ;
*!*		 Into Cursor gf_temp_csr ReadWrite

*!*		*-- Update local cursor in order that user has applied
*!*		*!* ******************** Removed 11/09/2015 *****************
*!*		*!* Insert Into (lcSearchResultsAlias) Select * From gf_temp_csr
*!*		Select (m.lcSearchResultsAlias) 
*!*		Append from (Dbf('gf_temp_csr')) 

		insert into (m.lcSearchResultsAlias) ;
			Select &lcOriginalFields. From (Select * &lcLongClauses. From (m.lcSearchEngineResultsAlias) Order By &lcMyOrderBy.) source
Endif

Set Collate To (m.lcCollate)

Select (m.lcSearchResultsAlias) 
Goto Top

Select (m.lnSelect)

myearwood1 avatar Aug 13 '21 22:08 myearwood1

Thanks @myearwood1 - I will try to find some time to update GoFish with this new code. I can;t promise when it will actualyl get released through Thor Check for Updates, will I'll do it when I have some free time one weekend (probable in Fall season).

mattslay avatar Aug 18 '21 16:08 mattslay

In MaxFrame we had it conditionally build indexes as the user clicked a column header. It is less time consuming than the safe select technique.

On Wed., Aug. 18, 2021, 12:44 p.m. Matt Slay, @.***> wrote:

Thanks @myearwood1 https://github.com/myearwood1 - I will try to find some time to update GoFish with this new code. I can;t promise when it will actualyl get released through Thor Check for Updates, will I'll do it when I have some free time one weekend (probable in Fall season).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mattslay/GoFish/issues/5#issuecomment-901266129, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABL36WYIFRGIEDBQHBCSAYDT5PPOZANCNFSM4FGGSK2A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

myearwood1 avatar Aug 18 '21 17:08 myearwood1

Since Matt (owner of this repo) passed away there is a new fork for GoFish. There is now also a new version in which this error got fixed. 👍 https://github.com/VFPX/GoFish

PatrickvonDeetzen avatar Aug 18 '22 17:08 PatrickvonDeetzen