Add documentation for SetQueryNotification

[chris-rossi]

Add Godoc documentation for the SetQueryNotification feature.

[codecov]

Codecov Report

Merging #524 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
+ Coverage   68.84%   68.86%   +0.01%     
==========================================
  Files          22       22              
  Lines        5056     5058       +2     
==========================================
+ Hits         3481     3483       +2     
  Misses       1369     1369              
  Partials      206      206

Impacted Files	Coverage Δ
mssql.go	82.72% <ø> (ø)	⬆️
conn_str.go	100% <0%> (ø)	⬆️

---

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 208c0a4...8638508. Read the comment docs.

[yukiwongky]

Only documentation (md files) should be in the doc folder. If this example test works as a standalone test, please put it in the root folder.

Since this example is targeting the SetQueryNotification, you should rename is to something like setquerynotification_example_text.go.

If you look at other *_examples_test.go files (e.g., datetimeoffset_example_test.go), you will see that instead of identifying is as package main, they use package mssql_test. That way if this file is in the root folder along with the src files, you will not get package clash (having 2 packages in the same directory). When godoc gets this example, they'll replace package mssql_test with package main so it looks like a proper test application.

Suggested change

	package main
	package mssql_test

It's also a good idea to add a .md file in the doc folder to show the users now this query notification feature works.

[chris-rossi]

Since the function has "Example" prefixing it's name it won't be run as a test. I have moved it from the doc folder though.

[yukiwongky]

Don't need to explicitly set the alias. It is mssql by default.

Suggested change

	mssql "github.com/denisenkom/go-mssqldb"
	"github.com/denisenkom/go-mssqldb"

[chris-rossi]

I kept it since it seems to be like this in other examples.

[yukiwongky]

You will get redefinition errors here if this file is in package mssql_test, since these variables are already defined in newconnector_example_test.go. You can just remove this whole block of variables.

[yukiwongky]

Is there any special setup needed for the Schema and Table for query notification to work? Perhaps add comments letting user know what setup is required?

[chris-rossi]

There is no special setup for the schema or table but query notifications must be setup on SQL Server for this example to work.

[yukiwongky]

What does rows contain in this case? Perhaps add a comment letting user know what is the expected output?

[yukiwongky]

Add newline.

[kardianos]

@chris-rossi and @yukiwongky Thank you for this documentation.

Upon examining this feature and how it is used, I would strongly like to alter the current implementation unless I'm totally missing something.

Recent database/sql package has a Conn.Raw function, which is our key to functionality like this. https://pkg.go.dev/database/sql?tab=doc#Conn.Raw

I would like to come up with an alternate API for using query notifications that do not involve bypassing the connection pool. First, is it required to set it on a Stmt? In SQL Server, a query batch is first class, not a Stmt. Second, I would like an API like:

package mssql

type QueryNotificationOptions {
    Message string
    Options string // map[string]string?
    Timeout time.Duration
}

// SetQueryNotification may be used on a *sql.Conn prior to a query to receive notifications
// on that query until the given timeout.
//
//    conn, err := db.Conn(ctx)
//    err = mssql.SetQueryNotifications(conn, opts)
//    rows, err := conn.QueryContext(ctx, `select * from notifications;`)
func SetQueryNotification(conn *sql.Conn, opts QueryNotificationOptions) error {
    return conn.Raw(func(ci interface{}) error {
        msc, ok := ci.(*mssql.Conn)
        if !ok {
            return fmt.Errorf("expected connection to be of type *mssql.Conn, got %T", ci)
        }
        msc.SetQueryNotification(opts)
    })
}

[chris-rossi]

@kardianos I don't see any reason why this wouldn't work. I think initially they designed it to be set in Stmt because it would be need to changed on a query to query basis but as long as they change the "NotifyId" (Message) for each query they want a subscription for. We would need to something that would clear the QueryNotificationOptions for the cases where they don't want a query notification subscription.

[kardianos]

@chris-rossi Then maybe the signature would be:

type Notification struct {...}
func (n *Notification) QueryContext(ctx context.Context, query string, ...interface{}) (*sql.Rows, error)
func (n *Notification) Cancel(ctx context.Context) error
func (n *Notification) ID() string
func SetQueryNotification(conn *sql.Conn, opts QueryNotificationOptions) (*Notification, error)

?

[chris-rossi]

I presume ID() will return the NotifyID. This looks good to me.