Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSSQL Query Support #57

Closed
tooolbox opened this issue Mar 18, 2021 · 14 comments
Closed

MSSQL Query Support #57

tooolbox opened this issue Mar 18, 2021 · 14 comments
Assignees
Labels
feature request Enhancement or feature request

Comments

@tooolbox
Copy link
Collaborator

I quite like the server you've put together, it checks a few boxes I'm interested in for a particular project.

I see you have PQ() for doing arbitrary Postgres queries. I need to support MSSQL; perhaps we could add a MSQ() function? Seems straightforward, basically copying lua/pquery/pquery.go and importing the appropriate driver.

Let me know what you think.

@tooolbox tooolbox changed the title Add MSQ MSSQL Query Support Mar 18, 2021
@xyproto xyproto self-assigned this Mar 19, 2021
@xyproto xyproto added the feature request Enhancement or feature request label Mar 19, 2021
@xyproto
Copy link
Owner

xyproto commented Mar 19, 2021

Thanks for reporting! It would make sense to support many different database systems, but I would ideally like to add a uniform interface. But for different databases, I guess it makes sense to add different interfaces.

I'll have to think a bit more about what the best design for this would be.

@tooolbox
Copy link
Collaborator Author

But for different databases, I guess it makes sense to add different interfaces.

I guess? I'm just imagining running some SQL and getting back a table of results, or the appropriate response depending on the type of query. I don't think it needs to be different at the Lua level, it's more that I imagine a separate Go implementation is needed for each DB because the driver will vary.

Anyway, you are probably thinking with all that.

Any thought of when you might take a crack at this--next week, a few months?

@xyproto
Copy link
Owner

xyproto commented Mar 20, 2021

Adding support should be quick, a week or two, but I might need to set up a MSSQL server just for testing.

Do you happen to know if this module is known to work well?

https://github.com/denisenkom/go-mssqldb

@tooolbox
Copy link
Collaborator Author

Adding support should be quick, a week or two, but I might need to set up a MSSQL server just for testing.

Nice :)

Do you happen to know if this module is known to work well?

Yes, that's what I've used in the past and it worked fine.

@xyproto
Copy link
Owner

xyproto commented Mar 22, 2021

I added a new MSSQL function to the main branch. Please test.

@tooolbox
Copy link
Collaborator Author

I added a new MSSQL function to the main branch. Please test.

Wow, great! Will let you know.

@tooolbox
Copy link
Collaborator Author

Still trying it out, but can confirm I can SELECT @@VERSION on my database.

@tooolbox
Copy link
Collaborator Author

Would be good to scan rows into 2-dimensional tables.

Perhaps this would work:

// StringSlices2table converts a slice of string slices to a Lua table
func StringSlices2table(L *lua.LState, sl [][]string) *lua.LTable {
	table := L.NewTable()
	for _, inner := range sl {
		sub := L.NewTable()
		for _, element := range inner {
			sub.Append(lua.LString(element)
		}
		table.Append(sub)
	}
	return table
}

And then in mssql.go you'll want to Scan into a []string instead of a string.

@tooolbox
Copy link
Collaborator Author

Two additional points:

  1. Should be able to pass arguments that get escaped to prevent SQL injection. Perhaps usage would be:
local result = MSSQL([[ SELECT r, g, b, a FROM [My Colors] WHERE name = @p1 ]], connstr, "mauve")
  1. When you use MSSQL() it appears that the keys in the resulting Lua table are textual, so if I have this Pongo2 template:
<ul>
{% for index, item in doquery() sorted %}
    <li>{{ index }}</li>
{% endfor %}
</ul>

I get this output:

1
10
11
12
13
2
3
4
5
6
7
8
9

@xyproto
Copy link
Owner

xyproto commented Mar 29, 2021

Those are pretty good suggestions, thanks!

I like the idea of keeping the MSSQL function as simple as possible, but the sort order is a concern, I agree.

I'll either update the function or add another one.

@tooolbox
Copy link
Collaborator Author

Great, thanks! I'm excited for this.

(For whatever you end up doing, I want to retract my earlier offhand code suggestion of StringSlices2table. Ideally when you perform a query on multiple columns, you can get back an outer table keyed numerically (row number) and each of the inner tables are keyed by the column name, so accessing the data becomes result[2]["my_column"] rather than result[2][4]. I haven't delved into the database/sql package enough to know how to do that but I know it should be possible.)

@tooolbox
Copy link
Collaborator Author

@xyproto so in my fork I have MSSQL() producing a 2-dimensional table with the proper Lua values (not all strings). The issue at this point is converting to map[string]interface{} before such things as passing to pongo2. This creates the string keys which sort incorrectly, as well as losing the nullity of something, although pongo2 may not have a concept of a nil, I haven't checked. Converting Lua tables to JSON in this way also omits keys which are present but have nil values.

I am not sure if the best way to proceed is by producing a map[interface{}]interface{} or if you have other thoughts. In the meantime, I'm still ironing out prepared statements/parameters.

@tooolbox
Copy link
Collaborator Author

tooolbox commented May 1, 2021

This is now mostly complete with #61

Lastly I will need to check the pongo2 sorting and do a PR to fix that if necessary.

@xyproto
Copy link
Owner

xyproto commented May 28, 2021

Thank you for the pull request and fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Enhancement or feature request
Projects
None yet
Development

No branches or pull requests

2 participants