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 Module does not support UPDATE statements #52831

Open
rdutch opened this issue May 3, 2019 · 6 comments
Open

MSSQL Module does not support UPDATE statements #52831

rdutch opened this issue May 3, 2019 · 6 comments
Labels
Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Feature new functionality including changes to functionality and code refactors, etc.
Milestone

Comments

@rdutch
Copy link
Contributor

rdutch commented May 3, 2019

The current (2019.2.0) mssql module does not support UPDATE statements, they return empty and no UPDATE happens. Example: UPDATE testtable set test1='Apples'

This missing functionality is very important to be able to update certain tables which are used for configuration settings as part of a application.

The current workaround is to use a power shell script.

Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: 1.6.0
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Apr  9 2019, 14:30:50)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.6.1810 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-957.12.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7.6.1810 Core
@rdutch
Copy link
Contributor Author

rdutch commented May 3, 2019

Microsoft SQL Server Management Studio						11.0.2100.60
Microsoft Analysis Services Client Tools						11.0.2100.60
Microsoft Data Access Components (MDAC)						6.3.9600.16384
Microsoft MSXML						3.0 6.0 
Microsoft Internet Explorer						9.11.9600.17031
Microsoft .NET Framework						4.0.30319.34014
Operating System						6.3.9600

@KChandrashekhar KChandrashekhar added the Feature new functionality including changes to functionality and code refactors, etc. label May 3, 2019
@KChandrashekhar KChandrashekhar added this to the Approved milestone May 3, 2019
@pere3
Copy link

pere3 commented Jun 4, 2019

I believe the main reason for that behavior is that tsql_query function only makes Cursor.execute() and Cursor.fetchall() calls:

def tsql_query(query, **kwargs):
    '''
    Run a SQL query and return query result as list of tuples, or a list of dictionaries if as_dict was passed, or an empty list if no data is available.

    CLI Example:

    .. code-block:: bash

        salt minion mssql.tsql_query 'SELECT @@version as version' as_dict=True
    '''
    try:
        cur = _get_connection(**kwargs).cursor()
        cur.execute(query)
        # Making sure the result is JSON serializable
        return loads(_MssqlEncoder().encode({'resultset': cur.fetchall()}))['resultset']
    except Exception as err:
        # Trying to look like the output of cur.fetchall()
        return (('Could not run the query', ), (six.text_type(err), ))

and for any queries that actually modifies the data you also need to call Connection.commit() or set mssql.autocommit: True pillar (which is not mentioned in docs btw)
also as mentioned by @rdutch INSERT/UPDATE queries return empty on Cursos.fetchall() which is fails try: inside tsql_query

i see how difficult it is to determine query type inside tsql_query to apply different flows, so maybe solution is to create another function like update_query which will follow execute-commit flow?

@stale
Copy link

stale bot commented Jan 8, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Jan 8, 2020
@sagetherage sagetherage added the Confirmed Salt engineer has confirmed bug/feature - often including a MCVE label Jan 9, 2020
@stale
Copy link

stale bot commented Jan 9, 2020

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Jan 9, 2020
@amalaguti
Copy link

@rdutch The issue is still present, I'm facing this problem now.
Here's my fix, as mentioned in the previous comments autocommit should be set to true, or do a commit() after execution of the query

`
def tsql_query(query, **kwargs):
try:
#cur = _get_connection(**kwargs).cursor()
conn = _get_connection(**kwargs)
conn.autocommit(True)
cur = conn.cursor()
cur.execute(query)

   # TODO: use cur.lastrowid to check if row was inserted and return True

`

Probably I will do a custom function specific to deal with INSERTS, to avoid changing def tsql_query

@floppy-disk
Copy link

I would like to do a pull request to fix this issue. I added a new function called "mssql.update_query" with the changes mentioned by @amalaguti last year. I spent hours trying to get this to work before finding this.

Is there a branch I should use for this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Feature new functionality including changes to functionality and code refactors, etc.
Projects
None yet
Development

No branches or pull requests

6 participants