[Home] [Downloads] [Search] [Help/forum]


Register forum user name Search FAQ

Gammon Forum

[Folder]  Entire forum
-> [Folder]  MUSHclient
. -> [Folder]  General
. . -> [Subject]  Question about SQL specifically nrows

Question about SQL specifically nrows

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page


Pages: 1 2  

Posted by Teclab85   (16 posts)  [Biography] bio
Date Tue 10 May 2016 01:13 AM (UTC)
Message
So according to the gammon api pages db:exec should not be broken out of because it will lock the db. I currently do, because I didn't read that part before when I made a script, so can someone explain what this is actually doing? My script seems to work just fine.

I am willing to rewrite the parts required if it is truly doing something malicious or if it is just a huge time sink, but other than that I just need to know if it is not worth the rewrite.
[Go to top] top

Posted by Nick Gammon   Australia  (22,973 posts)  [Biography] bio   Forum Administrator
Date Reply #1 on Tue 10 May 2016 02:08 AM (UTC)
Message
Where does it say that exactly? Can you quote it?

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

Posted by Teclab85   (16 posts)  [Biography] bio
Date Reply #2 on Tue 10 May 2016 02:11 AM (UTC)
Message
at http://www.gammon.com.au/scripts/doc.php?lua=db:nrows


Last line before the See also
"Note: You should let the iterator finish (that is, get to the end of the selected rows) otherwise the database remains locked. So, do not "break" out of the for loop."
[Go to top] top

Posted by Fiendish   USA  (2,514 posts)  [Biography] bio   Global Moderator
Date Reply #3 on Tue 10 May 2016 08:36 AM (UTC)
Message
See: http://www.mushclient.com/forum/bbshowpost.php?bbsubject_id=11423

https://github.com/fiendish/aardwolfclientpackage
[Go to top] top

Posted by Teclab85   (16 posts)  [Biography] bio
Date Reply #4 on Tue 10 May 2016 12:38 PM (UTC)

Amended on Tue 10 May 2016 01:43 PM (UTC) by Teclab85

Message
Fiendish, that didn't really answer the question, it just reinforced that it should not be done. Could you elaborate a little more?

Is returning out of a function similar to just breaking out of the loop? As in, if found I return from the loop that is iterating over the SQL call.
[Go to top] top

Posted by Nick Gammon   Australia  (22,973 posts)  [Biography] bio   Forum Administrator
Date Reply #5 on Tue 10 May 2016 10:38 PM (UTC)

Amended on Tue 10 May 2016 11:04 PM (UTC) by Nick Gammon

Message
A Lua iterator is designed to be called repeatedly inside a "for" loop. The function nrows returns such an iterator, after doing an SQLite3 "prepare" (to set up for doing a SELECT or similar). After doing a "prepare" you are supposed to "finalize" to clean up the prepared statement, unlock the database, and free resources.

What happens in this iterator is that, when it reaches the end of the requested set of data (that is, the attempt to get more data returns SQLITE_DONE) it does the cleanup, and then returns nil, which exits the for loop.

If you break out of the loop (ie. using break or return) then that code is not executed and the database remains locked.

Your options are:


  • Don't break out of the loop, but let it finish, ignoring the other rows you don't want.
  • Make the query such that it exits itself when your desired condition is reached. For example, if you want to stop after a certain item is found, make that part of the WHERE clause.
  • Do the prepare / execute / finalize yourself (see below).


http://www.gammon.com.au/scripts/doc.php?general=lua_sqlite3

You can:





Example:


require "tprint"

-- make in-memory database
db = sqlite3.open_memory()

-- make a table for testing
db:exec  [[
          CREATE TABLE mobs (name, class, hp);
          INSERT INTO mobs VALUES("Naga", "mage", 666);
          INSERT INTO mobs VALUES("Wolf", "beast", 42);
          INSERT INTO mobs VALUES("Guard", "warrior", 100);
       ]]

-- prepare a SELECT statement
local stmt = db:prepare ("SELECT * FROM mobs")

-- loop until we get everything
while true do

  local result = stmt:step ()

  -- exit loop if done
  if result == sqlite3.DONE then
    break
  end -- if done

  -- should have ROW result
  assert (result == sqlite3.ROW, "Row not found")

  -- get all values into a table
  local row = stmt:get_named_values()

  -- display them
  print (string.rep ("-", 20))
  tprint (row)
  
end -- while

-- done with this statement
stmt:finalize ()

-- finished with database
db:close ()


Now in this example, since we are handling the finalize ourselves, we can break out of that loop if we want to, because we will then execute the stmt:finalize () anyway. If you choose to do a return then you need to do the finalize before returning.

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

Posted by Nick Gammon   Australia  (22,973 posts)  [Biography] bio   Forum Administrator
Date Reply #6 on Tue 10 May 2016 11:02 PM (UTC)

Amended on Tue 10 May 2016 11:03 PM (UTC) by Nick Gammon

Message
To trim it down to the minimum, instead of this:


for row in db:nrows("SELECT * FROM mobs") do
  tprint (row)
end


Do this:


local stmt = db:prepare ("SELECT * FROM mobs")
local result = stmt:step ()  -- first read
while result == sqlite3.ROW do
  local row = stmt:get_named_values()
  tprint (row)
  result = stmt:step ()  -- next read
end -- while
stmt:finalize ()


It's slightly lengthier, but you have more control.

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

Posted by Fiendish   USA  (2,514 posts)  [Biography] bio   Global Moderator
Date Reply #7 on Tue 10 May 2016 11:27 PM (UTC)

Amended on Tue 10 May 2016 11:35 PM (UTC) by Fiendish

Message
Teclab85 said:

Fiendish, that didn't really answer the question, it just reinforced that it should not be done. Could you elaborate a little more?

I think it does sort of answer with the part that says "You can test this by doing something like...", though obviously it's not as good of an explanation as what Nick has here because it was just a report. Perhaps what is missing is a real life implication of this problem?

See here then also:
https://github.com/fiendish/aardwolfclientpackage/issues/165
(It says posted by me because it's a conversion from the old defunct googlecode repository. It's actually reported by another user.)

Quote:

Is returning out of a function similar to just breaking out of the loop? As in, if found I return from the loop that is iterating over the SQL call.
Yes. Same thing.

https://github.com/fiendish/aardwolfclientpackage
[Go to top] top

Posted by Nick Gammon   Australia  (22,973 posts)  [Biography] bio   Forum Administrator
Date Reply #8 on Wed 11 May 2016 05:58 AM (UTC)
Message
This technique could be usefully used when you know you will only have a single row returned. For example:


stmt = db:prepare ("SELECT COUNT(*) FROM mobs")
stmt:step ()  -- get first (and only) row
count = stmt:get_value (0)  -- get first column
stmt:finalize ()

print ("Count =", count)


In this case we know that a count will only ever be a single row, so this saves having a fake "for" loop, where we know in advance it will only ever have a single iteration.

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

Posted by Nick Gammon   Australia  (22,973 posts)  [Biography] bio   Forum Administrator
Date Reply #9 on Wed 11 May 2016 06:02 AM (UTC)
Message
Another example shows how you can get a single result from a SELECT, where you may or may not get any data:


stmt = db:prepare ("SELECT * FROM mobs WHERE name = 'Naga' ")
result = stmt:step ()
if result == sqlite3.ROW then
  tprint (stmt:get_named_values())
else
  print ("Not found")
end -- if
stmt:finalize ()

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

Posted by Teclab85   (16 posts)  [Biography] bio
Date Reply #10 on Wed 11 May 2016 01:32 PM (UTC)

Amended on Wed 11 May 2016 02:14 PM (UTC) by Teclab85

Message
Well I am trying to convert my stuff over to not breaking out of the loops, its a little hard because several tings use nested for loops to iterate through several different dbs.

I want to thank Nick and Fiendish for answering my question and helping me resolve it.


Quick note, by doing as I am supposed to and not breaking out of the for loops by whole script runs about 5 seconds slower and locks the mud up while its thinking.
[Go to top] top

Posted by Fiendish   USA  (2,514 posts)  [Biography] bio   Global Moderator
Date Reply #11 on Wed 11 May 2016 07:24 PM (UTC)

Amended on Wed 11 May 2016 07:25 PM (UTC) by Fiendish

Message
Teclab85 said:

Quick note, by doing as I am supposed to and not breaking out of the for loops by whole script runs about 5 seconds slower and locks the mud up while its thinking.

Whaaaat? What are you doing?

https://github.com/fiendish/aardwolfclientpackage
[Go to top] top

Posted by Nick Gammon   Australia  (22,973 posts)  [Biography] bio   Forum Administrator
Date Reply #12 on Wed 11 May 2016 07:42 PM (UTC)
Message
You can break out of the loops by using the techniques I described above. Five seconds sounds like a long time, however.

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

Posted by Teclab85   (16 posts)  [Biography] bio
Date Reply #13 on Wed 11 May 2016 07:44 PM (UTC)
Message
Fiendish I meant my client.

I will post the code snippits when my son goes down and see if I am just royally messing up with this or missing something.
[Go to top] top

Posted by Teclab85   (16 posts)  [Biography] bio
Date Reply #14 on Wed 11 May 2016 08:55 PM (UTC)

Amended on Thu 12 May 2016 12:14 AM (UTC) by Teclab85

Message
So this is a little snippit of the old code.

local loc = cp_mobs[tableNum].location
local mob_table_name= ''
local rows_counter= 1
local rows_counter_check= 1
 res, gmcparg = CallPlugin("3e7dedbe37e44942dd46d264","gmcpval","char")
 luastmt = "gmcpdata = " .. gmcparg 
 assert (loadstring (luastmt or "")) ()
 level = tonumber(gmcpdata.status.level) 
 min_level = level - 11 
 max_level = level + 11 
 if (min_level<0)then min_level=0 end
  if dbA:isopen() then
    query1 = string.format("select rooms.uid,"..
      " areas.name as areaName,"..
      " rooms.name as roomName"..
      " from rooms, areas"..
      " where areas.uid = rooms.area and rooms.name = %s",fixsql(cp_mobs[tableNum].location))
    for rows in dbA:nrows(query1) do
      rows_counter = rows_counter+ 1
    end--for
    for rows in dbA:nrows(query1) do
      rows_counter_check = rows_counter_check+1
      if dbkt:isopen() then
        query = string.format("select room_id, COUNT(*) as timeskilled,"..
          " name from mobkills "..
          "where room_id = %s "..
          " group by room_id, name "..
          "ORDER by timeskilled asc ", fixsql(rows.uid))
        for a in dbkt:nrows(query) do   
          roomTemp= a.room_id
          if string.lower(a.name) == string.lower(name) then
            roomNumber= roomTemp
            mob_table_name = a.name
            break       
          end --if
        end--for
      end--if
      for a, v in ipairs(areaLevel) do
        if rows.areaName == v.name then
          if v.minLevel ~=nil and v.maxLevel ~=nil then
            if v.minLevel > max_level or v.maxLevel<min_level then 
              rows_counter= rows_counter-1
              rows_counter_check = rows_counter_check-1
              break
            else
              if room_num_table ~= nil and #room_num_table > 0 then
                for i, v in ipairs (room_num_table) do
                  if (string.lower(v[2]) == string.lower(nameHolder) or v[1]== rows.uid and tableNum == tableNumHolder) then
                    return
                  end--if
                end--for
              end--if
                if mob_table_name~= '' then
                  makeTable(roomNumber, cp_mobs[tableNum].name, cp_mobs[tableNum].mobdead, true)
                  mob_index= 1
                  return
                end--if for the commented section below
                 makeTable2(tonumber(rows.uid), cp_mobs[tableNum].name, cp_mobs[tableNum].mobdead, false)
                 mob_index= 1
            end--if
          end--if
        end--if
      end--for
 else")
    end--for
  end--if
  if rows_counter_check == rows_counter then
    makeTable(-1, cp_mobs[tableNum].name, cp_mobs[tableNum].mobdead, false)
 
  end--if
 
  tableNumHolder= tableNum
end 


This is what I turned it into.
[Go to top] top

The dates and times for posts above are shown in Universal Co-ordinated Time (UTC).

To show them in your local time you can join the forum, and then set the 'time correction' field in your profile to the number of hours difference between your location and UTC time.


43,560 views.

This is page 1, subject is 2 pages long: 1 2  [Next page]

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page

Go to topic:           Search the forum


[Go to top] top

Quick links: MUSHclient. MUSHclient help. Forum shortcuts. Posting templates. Lua modules. Lua documentation.

Information and images on this site are licensed under the Creative Commons Attribution 3.0 Australia License unless stated otherwise.

[Home]


Written by Nick Gammon - 5K   profile for Nick Gammon on Stack Exchange, a network of free, community-driven Q&A sites   Marriage equality

Comments to: Gammon Software support
[RH click to get RSS URL] Forum RSS feed ( https://gammon.com.au/rss/forum.xml )

[Best viewed with any browser - 2K]    [Hosted at HostDash]