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.

fix statement.LimitN(0) will delete or update all data #1119

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix statement.LimitN(0) will delete or update all data #1119

wants to merge 2 commits into from

Conversation

yifhao
Copy link

@yifhao yifhao commented Oct 17, 2018

change LimitN from int to *int.

@yifhao
Copy link
Author

yifhao commented Oct 17, 2018

#1117

@codecov-io
Copy link

codecov-io commented Oct 17, 2018 •
edited
Loading

Codecov Report

Merging #1119 into master will decrease coverage by 0.01%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1119      +/-   ##
==========================================
- Coverage   54.64%   54.63%   -0.02%     
==========================================
  Files          42       42              
  Lines        7640     7649       +9     
==========================================
+ Hits         4175     4179       +4     
- Misses       2937     2940       +3     
- Partials      528      530       +2
Impacted Files Coverage Δ
session_iterate.go 80% <100%> (ø) ⬆️
session_delete.go 58.86% <20%> (-0.12%) ⬇️
statement.go 66.75% <47.05%> (-0.14%) ⬇️
session_update.go 52.69% <71.42%> (+0.15%) ⬆️
xorm.go 64.61% <0%> (-1.54%) ⬇️

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 1a7b211...da2991a. Read the comment docs.

@lunny lunny added this to the 0.8 milestone Oct 17, 2018
BetaCat0
BetaCat0 reviewed Oct 17, 2018
View reviewed changes
statement.go
@@ -689,7 +689,7 @@ func (statement *Statement) Top(limit int) *Statement {

// Limit generate LIMIT start, limit statement
func (statement *Statement) Limit(limit int, start ...int) *Statement {
statement.LimitN = limit
statement.LimitN = &limit
Copy link
Member

@BetaCat0 BetaCat0 Oct 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think statement.LimitN here will never get a nil val since you've already passed an non-pointer value within param limit. Furthermore, you should also write some cases to test your code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if limit > 0

Copy link
Member

@BetaCat0 BetaCat0 Oct 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe we still need an extra param to indicate that the value "0" in param limit is specified intentionally or not(considering using a bool param?).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a critical bug, it did delete all my table data, but i just want to delete none

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Frank-hust I will accept your PR, could you please resolve the conflict and add some tests for it?

mei-rune pushed a commit to mei-rune/xorm that referenced this pull request Apr 19, 2020
@lunny @ningmeng520
@yudppp @BetaCat0
fix statement.LimitN(0) will delete or update all data (go-xorm#1119)
6dfe337 
fix test

fix nil pointer

fix statement.Limit(0) will update or delete all data

fix bug when buffersize with iterate (go-xorm#941)

Merge branch 'master' into lunny/fix_buffer_iterate

Exclude schema from index name (#1505)

Merge branch 'master' into fix-schema-idx

SetExpr support more go types (#1499)

Improve tests

SetExpr support more go types

fix vet

fix drone lint

remove go1.10 test on drone

Reviewed-on: https://gitea.com/xorm/xorm/pulls/1499

fix vet

fix drone lint

remove go1.10 test on drone

Exclude schema from the index name

Co-authored-by: Guillermo Prandi <guillep2k@users.noreply.github.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Reviewed-on: https://gitea.com/xorm/xorm/pulls/1505

fix test

fix bug

fix bug when buffersize with iterate

SetExpr support more go types (#1499)

Improve tests

SetExpr support more go types

fix vet

fix drone lint

remove go1.10 test on drone

Reviewed-on: https://gitea.com/xorm/xorm/pulls/1499

fix vet

fix drone lint

remove go1.10 test on drone

Fix update with Alias (go-xorm#1455)

Co-authored-by: Guillermo Prandi <guillep2k@noreply.gitea.io>
Reviewed-on: https://gitea.com/xorm/xorm/pulls/941

fix update map with version (go-xorm#1448)

fix test

fix update map with version

SetExpr support more go types (#1499)

Improve tests

SetExpr support more go types

fix vet

fix drone lint

remove go1.10 test on drone

Reviewed-on: https://gitea.com/xorm/xorm/pulls/1499

fix vet

fix drone lint

remove go1.10 test on drone

Fix update with Alias (go-xorm#1455)

Reviewed-on: https://gitea.com/xorm/xorm/pulls/1448

Exclude schema from index name (#1505)

Merge branch 'master' into fix-schema-idx

SetExpr support more go types (#1499)

Improve tests

SetExpr support more go types

fix vet

fix drone lint

remove go1.10 test on drone

Reviewed-on: https://gitea.com/xorm/xorm/pulls/1499

fix vet

fix drone lint

remove go1.10 test on drone

Exclude schema from the index name

Co-authored-by: Guillermo Prandi <guillep2k@users.noreply.github.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Reviewed-on: https://gitea.com/xorm/xorm/pulls/1505

SetExpr support more go types (#1499)

Improve tests

SetExpr support more go types

fix vet

fix drone lint

remove go1.10 test on drone

Reviewed-on: https://gitea.com/xorm/xorm/pulls/1499

fix vet

fix drone lint

remove go1.10 test on drone

Fix update with Alias (go-xorm#1455)

Improve ci tests (#1477)

Rewrite Engine.QuoteTo() to accept multi-part identifiers (#1476)

Support local sql log (go-xorm#1338)

Fix go mod and update version (go-xorm#1460)

Move github.com/go-xorm/xorm to xorm.io/xorm (go-xorm#1459)

add support custom type Nullfloat64 (go-xorm#1450)

fix bug when query map condtion with no quote (go-xorm#1449)

Don't warn when bool column default is 1 but not true (go-xorm#1447)

* don't warn when bool column default is 1 but not true

* fix default case sensitive

Fix sync2 with custom table name (go-xorm#1445)

* fix sync2 with custom table name

* fix bug on postgres

* fix bug on postgres

fix bug when update with setexpr (go-xorm#1446)

add tidb tests on drone ci (go-xorm#1444)

improve sync2 (go-xorm#1443)

Fix wrong dbmetas (go-xorm#1442)

* add tests for db metas

* add more tests

* fix bug on mssql

Fix default value parse bugs (go-xorm#1437)

* fix default value

* fix default value tags

* fix postgres default

* fix default on postgres

* fix default on postgres

* fix mssql default

fix arg conversion (go-xorm#1441)

* fix arg conversion

* fix bugs

* fix bug on postgres

* use traditional positional parameters on insert into select

* remove unnecessary tests

upgrade core (go-xorm#1440)

add tests (go-xorm#1439)

add go1.13 tests on drone (go-xorm#1416)

Fix bug on insert where (go-xorm#1436)

* fix bug on insert where

* fix bug

* fix lint

fix bug when insert multiple slices with customize table name (go-xorm#1433)

* fix bug when insert multiple slices with customize table name

* fix tests on mssql

* fix tests

fix insert where with bool bug on mssql (go-xorm#1432)

fix setexpr missing big quotes (go-xorm#1431)

* fix setexpr missing big quotes

* fix tests

* fix tests

Add support subquery on SetExpr (go-xorm#1428)

* add support subquery on SetExpr

* fix tests

fix go mod (go-xorm#1427)

fix tests (go-xorm#1429)

Use strings.Builder instead of builder.StringBuilder (go-xorm#1417)

* use strings.Builder instead of builder.StringBuilder

* fix dependency

* fix dependency

Remove unuse get cols code (go-xorm#1413)

Add mssql ci test (go-xorm#1410)

* add mssql ci test

* fix drone test

Add insert select where support (go-xorm#1401)

Use drone new format (go-xorm#1388)

* use drone new format
fix get customize type bug (go-xorm#1382)

fix bugs (go-xorm#1375)

update drone (go-xorm#1374)

Add tests for get var (go-xorm#1305)

* add test for SQL get
* fix tests

fix error when get null var (go-xorm#890)

* fix error when get null var
* add support get for null var
* fix bug

Remove quotestr totally (go-xorm#1366)

* remove QuoteStr() totally

* update xorm.core -> v0.7.0

* update dialect Quote

remove QuoteStr() usage in dialects (go-xorm#1364)

document of FindAndCount() (go-xorm#1365)

remove QuoteStr() usage (go-xorm#1360)

Co-authored-by: yifhao <yifhao@tencent.com>
Co-authored-by: yifhao <1124210681@qq.com>
Co-authored-by: Guillermo Prandi <guillep2k@noreply.gitea.io>
Co-authored-by: Guillermo Prandi <guillep2k@noreply@gitea.io>
Co-authored-by: yudppp <yu.d.ppp@gmail.com>
Co-authored-by: BetaCat <outman99@hotmail.com>
Reviewed-on: https://gitea.com/xorm/xorm/pulls/1119
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BetaCat0 BetaCat0 BetaCat0 left review comments

@lunny lunny lunny left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
break enhancement
Projects
None yet
Milestone
0.8
Development

Successfully merging this pull request may close these issues.
5 participants
@yifhao @codecov-io @lunny @BetaCat0 @ningmeng520