fix issue #448 #1355

Merged
dawndiy merged 1 commits from oracle_join into master 2019-07-16 01:21:32 +00:00
dawndiy commented 2019-07-11 13:04:24 +00:00 (Migrated from github.com)

In xorm implementation, the statement can't know the struct joined in, so use '*' instead of the columns from target struct. But '*' can't be use in a subquery here (https://github.com/go-xorm/xorm/blob/v0.7.4/statement.go#L1160). Check the columnStr before use it .

In xorm implementation, the statement can't know the struct joined in, so use '\*' instead of the columns from target struct. But '*' can't be use in a subquery here (https://github.com/go-xorm/xorm/blob/v0.7.4/statement.go#L1160). Check the `columnStr` before use it .
codecov-io commented 2019-07-11 13:09:16 +00:00 (Migrated from github.com)

Codecov Report

Merging #1355 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1355      +/-   ##
==========================================
- Coverage    57.5%   57.49%   -0.01%     
==========================================
  Files          44       44              
  Lines        7833     7836       +3     
==========================================
+ Hits         4504     4505       +1     
- Misses       2773     2776       +3     
+ Partials      556      555       -1
Impacted Files Coverage Δ
statement.go 68.05% <0%> (-0.26%) ⬇️
xorm.go 69.69% <0%> (+1.51%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3e6c3a...67fbf85. Read the comment docs.

# [Codecov](https://codecov.io/gh/go-xorm/xorm/pull/1355?src=pr&el=h1) Report > Merging [#1355](https://codecov.io/gh/go-xorm/xorm/pull/1355?src=pr&el=desc) into [master](https://codecov.io/gh/go-xorm/xorm/commit/f3e6c3a26b8f7e70fee2751b885531c81bdfa928?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `0%`. [![Impacted file tree graph](https://codecov.io/gh/go-xorm/xorm/pull/1355/graphs/tree.svg?width=650&token=yB5nO1krEe&height=150&src=pr)](https://codecov.io/gh/go-xorm/xorm/pull/1355?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #1355 +/- ## ========================================== - Coverage 57.5% 57.49% -0.01% ========================================== Files 44 44 Lines 7833 7836 +3 ========================================== + Hits 4504 4505 +1 - Misses 2773 2776 +3 + Partials 556 555 -1 ``` | [Impacted Files](https://codecov.io/gh/go-xorm/xorm/pull/1355?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [statement.go](https://codecov.io/gh/go-xorm/xorm/pull/1355/diff?src=pr&el=tree#diff-c3RhdGVtZW50Lmdv) | `68.05% <0%> (-0.26%)` | :arrow_down: | | [xorm.go](https://codecov.io/gh/go-xorm/xorm/pull/1355/diff?src=pr&el=tree#diff-eG9ybS5nbw==) | `69.69% <0%> (+1.51%)` | :arrow_up: | ------ [Continue to review full report at Codecov](https://codecov.io/gh/go-xorm/xorm/pull/1355?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/go-xorm/xorm/pull/1355?src=pr&el=footer). Last update [f3e6c3a...67fbf85](https://codecov.io/gh/go-xorm/xorm/pull/1355?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Could you add some tests?

Could you add some tests?
dawndiy commented 2019-07-15 09:56:14 +00:00 (Migrated from github.com)

@lunny

Sure, but it can't be tested in a Golang UnitTest without a Oracle Server. So let me show examples here.

Basic structs:

type Admin struct {
	ID   int    `xorm:"ID"`
	Name string `xorm:"NAME"`
}

type Message struct {
	ID      int    `xorm:"ID"`
	Content string `xorm:"CONTENT"`
	AdminID int    `xorm:"ADMIN_ID"`
}

Demo snippets:

lst := []Message{}
s := engine.Table("MESSAGE").Join("LEFT", "ADMIN", "MESSAGE.ADMIN_ID=ADMIN.ID").Where("ADMIN.ID=:1", 1).Limit(1, 0)
s.Find(&lst)

Generate SQL:

current version

SELECT * FROM (SELECT *,ROWNUM RN FROM (SELECT * FROM "MESSAGE" LEFT JOIN ADMIN ON MESSAGE.ADMIN_ID=ADMIN.ID WHERE (ADMIN.ID=:1)) at WHERE ROWNUM <= 1) aat WHERE RN > 0 [1]

This SQL will cause a err from the Oracle Server like "ORA-00923: FROM keyword not found where expected"

with the PR

SELECT * FROM (SELECT at.*,ROWNUM RN FROM (SELECT * FROM "MESSAGE" LEFT JOIN ADMIN ON MESSAGE.ADMIN_ID=ADMIN.ID WHERE (ADMIN.ID=:1)) at WHERE ROWNUM <= 1) aat WHERE RN > 0 [1]

This SQL works well.

Demo2, normal Query with Limit

s := engine.Table("MESSAGE").Where("ADMIN_ID=:1", 1).Limit(1, 0)

Generate SQL

SELECT "ID", "CONTENT", "ADMIN_ID" FROM (SELECT "ID", "CONTENT", "ADMIN_ID",ROWNUM RN FROM (SELECT "ID", "CONTENT", "ADMIN_ID" FROM "MESSAGE" WHERE (ADMIN_ID=:1)) at WHERE ROWNUM <= 1) aat WHERE RN > 0 [1]

The current version and this PR will generate the same SQL, and works well.

Demo3, normal Query with JOIN

s := engine.Table("MESSAGE").Join("LEFT", "ADMIN", "MESSAGE.ADMIN_ID=ADMIN.ID").Where("ADMIN_ID=:1", 1)

Generate SQL

SELECT * FROM "MESSAGE" LEFT JOIN ADMIN ON MESSAGE.ADMIN_ID=ADMIN.ID WHERE (ADMIN_ID=:1) [1]

The current version and this PR will generate the same SQL, and works well.

@lunny Sure, but it can't be tested in a Golang UnitTest without a Oracle Server. So let me show examples here. ### Basic structs: ```go type Admin struct { ID int `xorm:"ID"` Name string `xorm:"NAME"` } type Message struct { ID int `xorm:"ID"` Content string `xorm:"CONTENT"` AdminID int `xorm:"ADMIN_ID"` } ``` ### Demo snippets: ```go lst := []Message{} s := engine.Table("MESSAGE").Join("LEFT", "ADMIN", "MESSAGE.ADMIN_ID=ADMIN.ID").Where("ADMIN.ID=:1", 1).Limit(1, 0) s.Find(&lst) ``` #### Generate SQL: current version ```sql SELECT * FROM (SELECT *,ROWNUM RN FROM (SELECT * FROM "MESSAGE" LEFT JOIN ADMIN ON MESSAGE.ADMIN_ID=ADMIN.ID WHERE (ADMIN.ID=:1)) at WHERE ROWNUM <= 1) aat WHERE RN > 0 [1] ``` This SQL will cause a err from the Oracle Server like **"ORA-00923: FROM keyword not found where expected"** with the PR ```sql SELECT * FROM (SELECT at.*,ROWNUM RN FROM (SELECT * FROM "MESSAGE" LEFT JOIN ADMIN ON MESSAGE.ADMIN_ID=ADMIN.ID WHERE (ADMIN.ID=:1)) at WHERE ROWNUM <= 1) aat WHERE RN > 0 [1] ``` This SQL works well. ### Demo2, normal Query with Limit ```go s := engine.Table("MESSAGE").Where("ADMIN_ID=:1", 1).Limit(1, 0) ``` #### Generate SQL ```sql SELECT "ID", "CONTENT", "ADMIN_ID" FROM (SELECT "ID", "CONTENT", "ADMIN_ID",ROWNUM RN FROM (SELECT "ID", "CONTENT", "ADMIN_ID" FROM "MESSAGE" WHERE (ADMIN_ID=:1)) at WHERE ROWNUM <= 1) aat WHERE RN > 0 [1] ``` The current version and this PR will generate the same SQL, and works well. ### Demo3, normal Query with JOIN ```go s := engine.Table("MESSAGE").Join("LEFT", "ADMIN", "MESSAGE.ADMIN_ID=ADMIN.ID").Where("ADMIN_ID=:1", 1) ``` #### Generate SQL ```sql SELECT * FROM "MESSAGE" LEFT JOIN ADMIN ON MESSAGE.ADMIN_ID=ADMIN.ID WHERE (ADMIN_ID=:1) [1] ``` The current version and this PR will generate the same SQL, and works well.

Thanks!

Thanks!
Sign in to join this conversation.
No description provided.