Skip to content
This repository has been archived by the owner on Sep 7, 2021. It is now read-only.
This repository is currently being migrated. It's locked while the migration is in progress.

Allow adding prefix for fields with extends tag #1284

Merged
merged 2 commits into from Jun 23, 2019
Merged

Allow adding prefix for fields with extends tag #1284

merged 2 commits into from Jun 23, 2019

Conversation

mskrip
Copy link
Contributor

@mskrip mskrip commented Apr 30, 2019

Added new syntax like extends('Prefix') to enable matching all
extended fields as PrefixFieldName.

Close #1270

@codecov-io
Copy link

codecov-io commented Apr 30, 2019

Codecov Report

Merging #1284 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1284      +/-   ##
==========================================
+ Coverage   57.37%   57.44%   +0.06%     
==========================================
  Files          44       44              
  Lines        7813     7837      +24     
==========================================
+ Hits         4483     4502      +19     
- Misses       2773     2777       +4     
- Partials      557      558       +1
Impacted Files Coverage Δ
tag.go 79.31% <100%> (+2.21%) ⬆️
engine.go 61.79% <100%> (+0.21%) ⬆️
rows.go 52.38% <0%> (-3.76%) ⬇️
session_exist.go 62.26% <0%> (-0.7%) ⬇️

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 f1b4f83...7842419. Read the comment docs.

@lunny
Copy link
Member

lunny commented May 1, 2019

Join Table Alias Name or prefix of the name, which is better?

SELECT <extends_name>.width, <extends_name>.height

instead of

SELECT <extends_name>width, <extends_name>height

@mskrip
Copy link
Contributor Author

mskrip commented May 1, 2019

IMO having just prefix is better here since it refers to what the column name of the query result should be changed to, it doesn't have to have anything in common with the table (the prefix I mean).

@mskrip
Copy link
Contributor Author

mskrip commented May 14, 2019

Sorry to bother you, but would I be able to get an ETA on this review? This is kind of a blocking change for me.

@lunny
Copy link
Member

lunny commented May 15, 2019

How to handle SizeClosed *Size xorm:"extends('')"` ?

@mskrip
Copy link
Contributor Author

mskrip commented May 16, 2019

Maybe as if there was no argument? So the same as just xorm:"extends" ?

@lunny
Copy link
Member

lunny commented May 17, 2019

@mskrip I think it's reasonable.

Added new syntax like `extends('Prefix')` to enable matching all
extended fields as 'PrefixFieldName'.

Close #1270
@mskrip
Copy link
Contributor Author

mskrip commented May 21, 2019

@lunny Added test for that case

@mskrip
Copy link
Contributor Author

mskrip commented Jun 11, 2019

@lunny ping

@lunny
Copy link
Member

lunny commented Jun 11, 2019

@mskrip will do last review tomorrow morning.

@mskrip
Copy link
Contributor Author

mskrip commented Jun 21, 2019

@lunny Okay, thank you

@lunny lunny merged commit 4c80660 into go-xorm:master Jun 23, 2019
@lunny
Copy link
Member

lunny commented Jun 23, 2019

@mskrip Thanks!

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

Successfully merging this pull request may close these issues.

Multiple extends of the same type
3 participants